[Dnsmasq-discuss] [dnsmasq] Errors found by static analysis of source code (Coverity)

[Simon Kelley]

On 06/02/13 06:08, Vladislav Grishenko wrote:
> Hi Simon,
> 
> We have a regression after aa608c84b499f75b692a1eb4120f7851f10912e4 Fix
> wrong syntax check.
> If lease time is set and valid, *cp points to \0 and so !*cp triggers bad
> dhcp-range error for valid case.
> Refer obvious patch attached.

Yes, got that.
> 
> Btw, why not to use strto(u)l funcs for number checking simplification?
> Refer alternative patch as well, probably other code places need it too.
No good reason, I guess I didn't have strol in my head when I wrote the
code.


HAVE_BROKEN_RTC patch applied too.

Cheers,

Simon.

> 
> Best Regards, Vladislav Grishenko
> 
>> -----Original Message-----
>> From: dnsmasq-discuss-bounces at lists.thekelleys.org.uk [mailto:dnsmasq-
>> discuss-bounces at lists.thekelleys.org.uk] On Behalf Of Simon Kelley
>> Sent: Tuesday, February 05, 2013 8:58 PM
>> To: Tomas Hozza
>> Cc: dnsmasq-discuss at thekelleys.org.uk
>> Subject: Re: [Dnsmasq-discuss] [dnsmasq] Errors found by static analysis
> of
>> source code (Coverity)
>>
>> On 04/02/13 10:24, Tomas Hozza wrote:
>>> Hello Simon.
>>>
>>> We at Red Hat are scanning a lot of open source packages with static
>>> analysis tool named Coverity. I have been scanning and reviewing group
>>> of network daemons where dnsmasq falls in, too.
>>>
>>> I scanned the latest dnsmasq-2.66-test13 source with Coverity version
>>> 6.5.1. It found 115 errors from which a lot of are just false
>>> positives or are not worth fixing. I wrote patches for issues that I
>>> think should be fixed. Please review and consider fixing these issues.
>>> I'm also including the Coverity scan log, so you can have a look at
>>> all errors.
>>>
>>> Coverity is also running a project where they allow open source
>>> project to be scanned for FREE. If you find it interesting you can
>>> find more information on http://scan.coverity.com/.
>>>
>>> If you have any questions about the scan or want to do more scanning,
>>> don't hesitate to write me back.
>>>
>>>
>>
>> More patches:
>>
>> 0018-RESOURCE_LEAK-CWE-404.patch
>> Taken, but only a problem if one malloc succeeds and a second fails - then
> we
>> leak the first block. I won't lose sleep over that.
>>
>> 0019-REVERSE_INULL-CWE-476.patch
>> Fixed. !cp should be !*cp
>>
>> 0020-STRING_OVERFLOW-CWE-120.patch
>> Not taken, same as 0001-STRING_OVERFLOW.....
>>
>> 0021-UNUSED_VALUE-CWE-563.patch
>> Taken. straightforward.
>>
>> 0022-USE_AFTER_FREE-CWE-416.patch
>> Taken. New code in 2.66test*
>>
>> 0023-USE_AFTER_FREE-CWE-416.patch
>> Taken, changed style of fix to match other code.
>>
>>
>>
>> A very worthwhile exercise, thanks Tomas.
>>
>> I've pushed the fixes into git.
>>
>>
>> Cheers,
>>
>> Simon.
>>
>>
>> _______________________________________________
>> Dnsmasq-discuss mailing list
>> Dnsmasq-discuss at lists.thekelleys.org.uk
>> http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss