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

[Jason Harris]

Hi Thorsten,

Great, thanks a lot for the patches!  I'll try to have a look at them 
over the next few days.

INDI is maintained by Jasem Mutlaq, one of the core KStars developers. 
He's subscribed to the list, so feel free to post INDI patches here for 
his review.

regards,
Jason

Thorsten Röder wrote:
> 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
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Kstars-devel mailing list
>> Kstars-devel at kde.org
>> https://mail.kde.org/mailman/listinfo/kstars-devel