mailing list archives
Re: [patch] fix strncpy() management
From: Vasily Kulikov <segoon () openwall com>
Date: Sun, 25 Aug 2013 22:27:42 +0400
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
All found cases were manually checked, some of them were false
positives. Some code doesn't need = '\0', but needs changes to the size
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
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
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
http://www.openwall.com - bringing security into open computing environments
Sent through the dev mailing list
Archived at http://seclists.org/nmap-dev/