Re: [patch] fix strncpy() management

[Vasily Kulikov <segoon () openwall com>]

On Fri, Aug 16, 2013 at 21:09 -0700, David Fifield wrote:
> On Tue, Aug 13, 2013 at 12:04:36PM +0400, Vasily Kulikov wrote:
> > The patch fixes several strncpy() calls to properly set '\0' in the end
> > of the string.  The bugs were found with the help of the following
> > coccinelle script:
> >
> > All found cases were manually checked, some of them were false
> > positives.  Some code doesn't need = '\0', but needs changes to the size
> > argument.
> >
> > strncpy() from libpcap/ need additional review as it looks like several
> > strncats deliberately don't add zero at the end of the string to satisfy
> > setsockopt/getsockopt API.
>
> Thank you for doing this check. Rather than manually zero the last byte,
> it were better to use the Strncpy function from nbase.

I wanted the changes to be consistent with the surrounding code.

> Any changes to libpcap and libpcre, please report to their upstream
> maintainers.
>
> https://github.com/mcr/libpcap/blob/master/README
> http://bugs.exim.org/buglist.cgi?product=PCRE&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED

OK, will do it.

> I don't understand this libpcre change:
>
> >  addmessage = " at offset ";
> >  addlength = (preg != NULL && (int)preg->re_erroffset != -1)?
> > -  strlen(addmessage) + 6 : 0;
> > +  strlen(addmessage) + 7 : 0;
>
> I'm guessing you changed 6 to 7 to be one greater than the "%-6d" in the
> following sprintf. But the 6 is a minimum width, not a maximum. It looks
> like it can still overflow if re_erroffset is big enough.
>
>   if (addlength > 0 && errbuf_size >= length + addlength)
>     sprintf(errbuf, "%s%s%-6d", message, addmessage, (int)preg->re_erroffset);

Hmm, you're right.  Will fix that too and send it to the libpcre
community.

Thanks!

-- 
Vasily Kulikov
http://www.openwall.com - bringing security into open computing environments
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/