[Kstars-devel] Suspicious Code - Need Review - Patch

Wed Apr 26 23:19:20 CEST 2006

hmmm, with the patch this time.

Thorsten

> -----Original Message-----
> From: Thorsten Röder [mailto:thorsten.roeder at weihenstephan.org] 
> Sent: Wednesday, April 26, 2006 11:18 PM
> To: kstars-devel at kde.org
> Cc: 'Thorsten Röder'
> Subject: Suspicious Code - Need Review
> 
> Hi,
> 
> Christoph Bartoschek did a great job
> finding a lots of suspicious lines of code in 3.5.2 and 
> posted these issues on the kde-core-devel list.
> 
> I tried to fix the mentioned problems in parts that "belongs" 
> to kstars.
> Please have a look at the attached patch, would be great if 
> someone can give me feedback about this, so I can commit that 
> if its ok.
> 
> Question: How to deal with the other problems "belonging" to 
> the INDI library itself? Is this library an external library 
> maintained by someone not on this list? Or is the INDI 
> library developed with kstars?
> 
> 
> These are/were the problems Christoph Bartoschek found:
> 
> ___ IN KSTARS___, should be fixed with the attached patch:
> 
> > - kstars/kstars/fitshistogram.cpp:148
> > 
> > If binSize is 0 but buffer is not NULL, then line 144 is 
> not executed 
> > and line 148 has a division by 0.
> > 
> Fixed.
> > - kstars/kstars/fitsprocess.cpp:147
> > 
> > Is narray leaking here?  If yes, why not using std::vector?
> >
> Fixed.
> > - kstars/kstars/indidevice.cpp:874
> > - kstars/kstars/indidevice.cpp:885 (similar)
> > - kstars/kstars/indidevice.cpp:898 (similar)
> > 
> > In line 872 you delete pp and use it here again.
> > 
> Fixed. PLEASE INVESTIGATE PATCH.
> > - kstars/kstars/fitsviewer.cpp:308
> > 
> > If buffer is != NULL here, than the memory is leaking here. 
> > std::vector is beffer for such tasks.
> > 
> Fixed.
> > - kstars/kstars/indistd.cpp:492
> > 
> > Use delete [] here or better a std::string or QString for 
> tempPrefix.
> > 
> Fixed.
> > - kstars/kstars/kstarsdata.cpp:553
> > 
> > If ok is true but nn != 2, then you delete seg in line 546. 
> > But you use it again in line 553.
> > 
> Fixed. PLEASE INVESTIGATE PATCH.
> > ------------------------------------
> > Problems involving the NULL pointer:
> > ------------------------------------
> > 
> > - kstars/kstars/kswizard.cpp:143
> > 
> > If the if condition in line 134 is false, then line 143 crashes.
> > 
> Fixed. PLEASE INVESTIGATE PATCH.
> > -----------------------------------------------------------------
> > Lines where boolean expressions are used in non-boolean contexts:
> > 
> > I suspect that at least the lines marked with !!! are bugs
> > -----------------------------------------------------------------
> > 
> > - kstars/kstars/kstarsdata.cpp:1663, 1675
> Fixed.
> 
> 
> 
> ___ IN INDI library___, not yet fixed:
> > - kstars/kstars/indi/apmount.cpp:657
> > - kstars/kstars/indi/apmount.cpp:658
> >
> > tmtexts and tmtp have only 1 byte allocated, but 4 bytes (32bit
> > platforms) are assigned here. Why not using std::vector and 
> > std::string for such tasks? Is the memory also leaking in 
> lines 667, 
> > 668?
> > 
> > - kstars/kstars/indi/apogee/ApnCamera.cpp:983
> > 
> > If no case is selected in line 967, then RegVal is 
> uninitialized here.
> > 
> > - kstars/kstars/indi/apogee/ApogeeUsbLinuxForKernel.cpp:354
> > 
> > retval is used uninitialized.
> > 
> > - kstars/kstars/indi/apogee/ApogeeUsbLinuxForKernel.cpp:367
> > 
> > Success is not set, if the loop in line 348 is not entered.
> > 
> > - kstars/kstars/indi/v4ldriver.cpp:662
> > 
> > Use delete [] here.  Or better a std::vector<unsigned char>. 
> > This way you prevent the memory leaks in lines 625 and 641.
> > 
> > - kstars/kstars/indi/v4lphilips.cpp:602
> > 
> > The index 5 is out of bounds of this array.
> > 
> > - kstars/kstars/indi/sbigccd.cpp:567
> > - kstars/kstars/indi/sbigccd.cpp:590
> > - kstars/kstars/indi/sbigccd.cpp:578
> > - kstars/kstars/indi/sbigccd.cpp:559 (only fitsData)
> > 
> > fitsData and compressedData are leaking memory here. Why not using 
> > std::vector<unsigned char>?
> > 
> > - kstars/kstars/indi/sbigccd.cpp:602
> > - kstars/kstars/indi/apogee_ppi.cpp:679 (similar)
> > 
> > After this function finished. imageB.blob points to freed 
> memory. Are 
> > there memory leaks, when the function returns to early in 
> the second 
> > case?
> > 
> > - kstars/kstars/indi/fli/libfli-mem.c:75
> > 
> > Buffer overflow. In line 68 num is set to 2*allocated.total. 
> > Then allocated.pointers gets a buffer for num pointers in 
> line 70. In 
> > line 75 you start at  position allocate.total and overwrite 
> num (= 2 *
> > allocated.total) elements with 0. The last allocated.total 
> elements do 
> > not belong to the buffer.
> > 
> > - kstars/kstars/indi/fli/libfli-filter-focuser.c:467
> > 
> > abs looses precision here. Maybe you want to use labs.
> > 
> > - kstars/kstars/indi/fli_ccd.c:898
> > - kstars/kstars/indi/fli_ccd.c:908
> > 
> > The memory pointed to by fitsData and compressedData is 
> leaking here.
> > 
> >  - kstars/kstars/indi/temmadriver.c:73
> >  
> >  Buffer overflow.  PortT->text has place for 10 chars, but strcpy 
> > copies
> >  11 into it. Note that strcpy adds the \0.
> > 
> >  kstars/kstars/indi/celestronprotocol.c:562
> >  
> > The loop is executed at most once.
> > 
> > - kstars/kstars/indi/webcam/v4l2_base.cpp:1116
> > 
> > num_ctrls is leaking here.
> >
> TR BTW: numbers is leaking here too.
> > -----------------------------------------------------------------
> > Lines where boolean expressions are used in non-boolean contexts:
> > 
> > I suspect that at least the lines marked with !!! are bugs
> > -----------------------------------------------------------------
> > 
> > - kstars/kstars/indi/apogee/CameraIO_LinuxPPI.cpp:251, 270, 300
> > - kstars/kstars/indi/apogee/CameraIO_LinuxPCI.cpp:301, 320, 348
> > - kstars/kstars/indi/apogee/CameraIO_LinuxISA.cpp:247, 266, 294
> 
> Thorsten
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kstars_suspicious_3.5.2.diff
Type: application/octet-stream
Size: 5072 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kstars-devel/attachments/20060426/eb07a843/attachment.obj 