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

[Vladislav Grishenko]

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.

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.

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dhcp-range-lease-fix.patch
Type: application/octet-stream
Size: 366 bytes
Desc: not available
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20130206/e3d4228e/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dhcp-range-lease-fix-alt.patch
Type: application/octet-stream
Size: 1121 bytes
Desc: not available
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20130206/e3d4228e/attachment-0001.obj>