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
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/


  By Date           By Thread  

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