Re: Revised patch for intf-win32.c in NMAP-6.01

[David Fifield <david () bamsoftware com>]

On Fri, Nov 23, 2012 at 08:59:36AM -0800, Bill Parker wrote:
>    In reviewing intf-win32.c in 'nmap-6.01/libdnet-stripped/src',
> per David Fifield, it would appear that '_ifcombo_add' is not
> called by any other routine in the NMAP-6.01 source tree (via a
> grep -r "_ifcombo_add" *) from the nmap-6.01 top level directory.
>
> I would agree that _ifcombo_add should return a value indicating
> failure or success, but if there is no corresponding call to
> _ifcombo_add in the NMAP-6.01 source tree, should this code even
> included, given that intf-win32.c is not even compiled under
> linux or *BSD when a 'make' or 'make all' is issued?

_ifcombo_add is called by _refresh_tables in the same file, which is in
turn called by important functions like intf_loop.

The file is compiled only on Windows. Please make sure you test on
Windows before submitting changes to Windows-only files.

> Here is a revised patch file for intf-win32.c to eliminate the
> fprintf calls in the event realloc() or malloc() returns NULL:

I'm sorry, we can't accept this patch. Just returning silently from the
function is not acceptable. As written, if malloc evern returns NULL,
the program will crash right away at the line
        ifc->idx[ifc->cnt].ipv4 = ipv4_idx;
The patched code bails out of the function, and so the crash will happen
somewhere else. Crashing in the first case is not good behavior, but the
patch will only make a problem harder to debug.

I can think of two decent ways to deal with it:
1. Just call abort if malloc fails.
2. Make _ifcombo_add return an error code, and have the calling function
   test it and behave accordingly.

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