Re: nmap-5.20 on x86_64 Segmentation fault

[Gunnar Lindberg <Gunnar.Lindberg () chalmers se>]

Yes!

Thanks,

        Gunnar

> From bmenrigh () ucsd edu Sun Jan 24 21:57:19 2010
> From: Brandon Enright <bmenrigh () ucsd edu>
> To: Gunnar Lindberg <gunnar.lindberg () chalmers se>
> CC: "nmap-dev () insecure org" <nmap-dev () insecure org>, "bmenrigh () ucsd edu"
>       <bmenrigh () ucsd edu>
> Date: Sun, 24 Jan 2010 21:57:51 +0100
> Subject: Re: nmap-5.20 on x86_64 Segmentation fault
> Message-ID: <20100124205751.3af8f9be () spoke wan>
> References: <201001241148.o0OBmX81012080 () grunert cdg chalmers se>
>       <201001242005.o0OK5HmD021206 () grunert cdg chalmers se>
> In-Reply-To: <201001242005.o0OK5HmD021206 () grunert cdg chalmers se>

> My comments inline.  My proposed patch attached.

> On Sun, 24 Jan 2010 21:05:17 +0100 (MET) or thereabouts Gunnar Lindberg
> <Gunnar.Lindberg () chalmers se> wrote:

> > Sorry to have confused people, but I was quite confused myself.
> >
> > This is not a x86_64 vs i386 thing.
> >
> > It's the IPv6 resolvers in /etc/resolv.conf
> > Behavior is the same on i386 as on x86_64.

> Indeed, many of us are using x86_64 almost exclusively.  I've always
> been pleasantly surprised that we don't run into more size/endian
> issues across all of the platforms we compile and run on.

> > nmap-5.20/nmap_dns.cc:
> > static void parse_resolvdotconf() {
> >   char ipaddr[16];
> >   ...
> >   if (sscanf(tp, "nameserver %65s", ipaddr) == 1)
> > add_dns_server(ipaddr); ...
> > }
> >
> > I tried "char ipaddr[128];" instead.

> Good find.  This code is pretty broken.  We're reading in
> "nameserver ...." lines and assuming that all of the name servers will
> be IPv4 addresses and not names or IPv6 addresses but then we tell
> sscanf() it can read 65 chars.  We later treat what we read in as a
> hostname or IP and pass it through our resolve() routine in tcpip.cc

> The correct thing to do in this function would be to increase ipaddr[]
> like you did and adjust our %65s passed to sscanf() so it can't smash
> our stack like it was doing.

> In my proposed patch I also change the name from ipaddr[] to nsrvr[] so
> that it is more clear that we're not guaranteed to be reading an IP.

> > No crash but:
> >
> >     Socket troubles: Address family not supported by protocol
> >     nmap: nsock_core.c:1163: nsp_add_event: Assertion `nse->iod->sd
> > > = 0' failed. Abort

> Indeed, Nsock doesn't do IPv6 yet.

> > As a workaround, skip IPv6 resolvers:
> >     if ((sscanf(tp, "nameserver %65s", ipaddr) == 1)
> > && !strchr(ipaddr, ':')) add_dns_server(ipaddr);

> Yeah, we should work around this rather than crash.  I think a more
> appropriate place is actually in add_dns_server() as that routine is
> the central function all the different ways you can define a DNS
> server wind up calling.  Also, it's what creates the struct
> sockaddr_storage for the nameserver.

> My proposed patch handles IPv6 this way:

> +    // Make sure we stick to IPv4 for now
> +    if (addr_len > sizeof(struct sockaddr_in)) {
> +      if (o.debugging) log_write(LOG_STDOUT, "mass_rdns: Skipping IPv6 DNS server %s\n", hostname);
> +      continue;
> +    }
> +

> Which results in this output:

> mass_rdns: Using DNS server 129.16.1.53
> mass_rdns: Using DNS server 129.16.2.53
> mass_rdns: Skipping IPv6 DNS server 2001:6b0:2:1::53
> mass_rdns: Skipping IPv6 DNS server 2001:6b0:2:2::53


> Thanks for your report and your help tracking this issue down.  I'll
> commit a fix when David has had a chance to think this over.

> Brandon
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/