Re: [nmap-svn] r32524 - nmap

[David Fifield <david () bamsoftware com>]

On Mon, Nov 25, 2013 at 06:35:49PM +0000, commit-mailer () nmap org wrote:
> Author: dmiller
> Date: Mon Nov 25 18:35:49 2013
> New Revision: 32524
>
> Log:
> Fix buffer overflow in parse_resolvconf()
>
> String ipaddr was allocated without allowing space for the null
> terminator, resulting in a 1-byte overflow. Caught with clang
> -fsanitize=address
>
> Also, fmt was being initialized with sizeof(ipaddr), which happened to
> be correct, but should not necessarily be so. We don't care about the
> size of the structure, but rather the length of an address in string
> notation.
>
> Modified:
>    nmap/nmap_dns.cc
>
> Modified: nmap/nmap_dns.cc
> ==============================================================================
> --- nmap/nmap_dns.cc  (original)
> +++ nmap/nmap_dns.cc  Mon Nov 25 18:35:49 2013
> @@ -986,7 +986,7 @@
>    FILE *fp;
>    char buf[2048], *tp;
>    char fmt[32];
> -  char ipaddr[INET6_ADDRSTRLEN];
> +  char ipaddr[INET6_ADDRSTRLEN+1];
>  
>    fp = fopen("/etc/resolv.conf", "r");
>    if (fp == NULL) {
> @@ -994,8 +994,7 @@
>      return;
>    }
>  
> -  /* Customize a sscanf format to sizeof(ipaddr). */
> -  Snprintf(fmt, sizeof(fmt), "nameserver %%%us", (unsigned int) sizeof(ipaddr));
> +  Snprintf(fmt, sizeof(fmt), "nameserver %%%us", INET6_ADDRSTRLEN);
>  
>    while (fgets(buf, sizeof(buf), fp)) {
>      tp = buf;

Good find here. I think what you actually want, though, is
  char ipaddr[INET6_ADDRSTRLEN];
  Snprintf(fmt, sizeof(fmt), "nameserver %%%us", INET6_ADDRSTRLEN-1);
INET6_ADDRSTRLEN already includes space for the null byte. RFC 2553 has
INET6_ADDRSTRLEN == 46, which is one more than
strlen("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255").

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