Home page logo

nmap-dev logo Nmap Development 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
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

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


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


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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]