Re: Buffer overflow in Nmap when using -ox - on a /8 scan

[Henri Doreau <henri.doreau () gmail com>]

2014-01-27 Henri Doreau <henri.doreau () gmail com>:
> 2014-01-27 Daniel Miller <bonsaiviking () gmail com>:
> > On 01/26/2014 10:46 AM, Daniel Miller wrote:
> > > On Sat, Jan 25, 2014 at 4:25 PM, Henri Doreau <henri.doreau () gmail com>
> > > wrote:
> > > > Hi,
> > > >
> > > > 2014-01-25 Jacek Wielemborek <d33tah () gmail com>:
> > > > > Hi,
> > > > >
> > > > > I just found a potentially interesting error. While experimenting with
> > > > > Nmap, I
> > > > > managed to get this strange error on Nmap 6.40 from Fedora 20:
> > > > >
> > > > > [22:46:39][/tmp][134] $ nmap localhost/8 --min-rate 100000  -ox - -sT
> > > > > [...]
> > > >
> > > > Huh?! What is "-sT --min-rate 100000"?
> > > >
> > > > It looks like the error comes from FD_ISSET, because you forced nmap
> > > > to open sockets beyond FD_SETSIZE. What we could have is a
> > > > CHECKED_FD_ISSET, that would abort() just like CHECK_FD_SET if the
> > > > socket # is greater than FD_SETSIZE. That would make the crash a bit
> > > > nicer but wouldn't essentially change anything...
> > > It looks like this check is supposed to fail, since it does without
> > > optimization (search Jacek's message for "FD_SETSIZE"). So the
> > > compiler is optimizing out the CHECKED_FD_SET check?
> > >
> > > Dan
> > > .
> > Correction: It's not an optimization difference, it's a -D_FORTIFY_SOURCE
> > difference. In the first (optimized) case, the code runs afoul of a fortify
> > bounds check when executing FD_ISSET, so it never gets to the checks
> > involved in CHECKED_FD_SET on the lines below it. In the unoptimized
> > version, -D_FORTIFY_SOURCE doesn't apply, so the code gets to Nmap's own
> > checks and aborts there.
> Yes, I think you're right.
>
> > The cleanest solution would seem to be Henri's suggested CHECKED_FD_ISSET,
> > but since FD_ISSET returns an int, I don't see how it could be properly
> > wrapped in the do{}while(0) and still checked. I think an explicit check
> > would be best, right after the assert(sd >= 0);
> >
> > Dan
> A static inline function would be an option, but I'd then like to have
> CHECKED_FD_SET turned into one as well for the sake of consistency.
> What do you think? External check is OK too, but there would be a
> couple other ones to add.
>
> Regards
>
> --
> Henri

Answering to my own post: how do you feel about this patch (attached)?

-- 
Henri

Attachment: check_fd_ops.patch
Description:

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