Re: Janitorial patch re: strtol

[David Fifield <david () bamsoftware com>]

On Fri, May 07, 2010 at 11:20:15PM -1000, William Pursell wrote:
> I'm new to the list, so I don't know if this sort
> of patch is welcome or considered pure churn, but I was
> poking around in the code and spotted an unnecessary
> error check.  In the following, there's no need to check
> if *s is a digit; if it's not, then strtol will set
> tail to it and the second check will catch it.

This kind of patch is most welcome and I wish we had more of them. I
like the idea of your patch. However there are cases where a string can
start with a non-digit and still be parsed without complaint by strtol,
namely when the string starts with whitespace or a sign. This patch lets
you use host specifications like "192.168.0.0/+24" or "192.168.0.0/ 24".

Maybe it makes sense to support these. I just found out my Epiphany
browser lets me go to http://nmap.org:+80/. Then again, browsers let you
do nonsense like http://1074628148/, which is suprising to users. (See
how the server at scanme.nmap.org rejects the "Host: 1074628148"
header.)

I think I prefer to reject weird netmasks like this. This error handling
was modeled on the parse_long function in ncat/utils.c, which rejects
signs and digits (http://seclists.org/nmap-dev/2009/q2/351). Using such
a function here would be better because the error handling could be
combined.

So I'm going to ask you to expand your patch somewhat, following these
steps.
        * Move parse_long from ncat/utils.c to nbase/nbase_misc.c.
        * Change the netmask parsing to use parse_long.
parse_long is used a lot in Ncat, particularly in the HTTP parser where
the protocol doesn't allow any non-digits in number in certain places.
It will probably be useful in other programs so nbase is a good place
for it. By the way, I was never thrilled with the name parse_long, so if
you have a better idea please suggest it.

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