Nmap Development mailing list archives
Re: [PATCH] Prevent useless buffer allocs and unpredictable payload lengths
From: David Fifield <david () bamsoftware com>
Date: Tue, 30 Jun 2009 13:51:14 -0600
On Wed, Jun 24, 2009 at 09:44:39AM +0200, Luis M. wrote:
Hi. While working on Nping I found a small bug in Nmap that could be fixed quite easily. The bug is related to option --data-length which allows the user to include payloads with random data in the packets that are sent by nmap. The code in nmap_main() that parses option --data-lenght is the following: } else if (optcmp(long_options[option_index].name, "data-length") == 0) { o.extra_payload_length = atoi(optarg); if (o.extra_payload_length < 0) { fatal("data-length must be greater than 0"); } else if (o.extra_payload_length > 0) { o.extra_payload = (char *) safe_malloc(o.extra_payload_length); get_random_bytes(o.extra_payload, o.extra_payload_length); } } Vars extra_payload_length extra_payload are defined in NmapOps.h as: - int extra_payload_length; - char *extra_payload; So the problem with all this is that we are allowing the user to call nmap like this: sudo nmap localhost --data-length 999999999
This is a good idea. I think you can simplify the implementation a bit. Don't use the aux variable and just do your calculations in o.extra_payload_length. Keep o.extra_payload_length an int; you're already doing bounds checking so you don't need to use a smaller data type. Be careful about indentation. The mix of spaces and tabs leads to this with 8-space tabs: + aux = atoi(optarg); + if (aux < 0 || aux > MAX_PAYLOAD_ALLOWED ) + fatal("data-length must be between 0 and %d", MAX_PAYLOAD_ALLOWED); + o.extra_payload_length=(u16)aux; + o.extra_payload = (char *) safe_malloc(o.extra_payload_length); + get_random_bytes(o.extra_payload, o.extra_payload_length); Either copy the tab/space mix from surrounding lines exactly, or expand all the tabs to spaces so indentation is uniform. But if you adjust the indentation, always do it in a separate commit from any functional changes. With those modifications, this is good to commit. David Fifield _______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://SecLists.Org
Current thread:
- [PATCH] Prevent useless buffer allocs and unpredictable payload lengths Luis M. (Jun 24)
- Re: [PATCH] Prevent useless buffer allocs and unpredictable payload lengths David Fifield (Jun 30)