Re: [patch] fix strncpy() management

[David Fifield <david () bamsoftware com>]

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.

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

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);

David Fifield
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/