[PATCH] Prevent useless buffer allocs and unpredictable payload lengths (original) (raw)

nmap-dev logo

Nmap Developmentmailing list archives

From: "Luis M." <luis.mgarc () gmail com>
Date: Wed, 24 Jun 2009 09:44:39 +0200

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:

So the problem with all this is that we are allowing the user to call nmap like this:

sudo nmap localhost --data-length 999999999

letting nmap_main() allocate 999999999 bytes (most of them useless!) and then passing that value directly to build_XXX_raw() functions.

For example, calls in scan_engine.cc like:

packet = build_udp_raw(&o.decoys[decoy], hss->target->v4hostip(), o.ttl, ipid, IP_TOS_DEFAULT, false, o.ipoptions, o.ipoptionslen, sport, pspec->pd.udp.dport, o.extra_payload, o.extra_payload_length, &packetlen);

are passing o.extra_payload_length without any checks. Fortunately, in those functions, payload length is defined as "u16 datalen". The "conversion" performed by my compiler (gcc 4.2.4) consists of keeping the 16 less significant bits stripping everything else so when whe pass "12345678" (0x00BC614E), build_udp_raw() gets 0x614E (24910). This gets worse when the supplied value is something like 286392319 (0x1111FFFF) becuase in that case build_udp_raw() gets 0xFFFF (65535), and, as we all know, no IP packet can carry payloads bigger than 65535-20, so we get this:

nmap: scan_engine.cc:814: void UltraProbe::setIP(u8*, u32, const probespec*): Assertion `iplen == (u32) (extension ({ register unsigned short int __v, __x = (ipv4->ip_len); if (__builtin_constant_p (__x)) __v = ((((__x) >> 8) & 0xff) | (((__x) & 0xff) << 8)); else asm ("rorw $8, %w0" : "=r" (__v) : "0" (__x) : "cc"); __v; }))' failed. Aborted

I attach a patch that should solve the problem by checking user-supplied value before allocating the buffer, and changing the type of extra_payload_length so we don't get surprises when converting from int to u16.

Index: nmap.cc

--- nmap.cc (revision 13870) +++ nmap.cc (working copy) @@ -554,7 +554,7 @@ int nmap_main(int argc, char *argv[]) { char *p, *q; - int i, arg; + int i, aux, arg; long l; unsigned int targetno; FILE *inputfd = NULL, *excludefd = NULL; @@ -900,13 +900,12 @@ o.setVersionTrace(true); o.debugging++; } 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); - } + 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); } else if (optcmp(long_options[option_index].name, "send-eth") == 0) { o.sendpref = PACKET_SEND_ETH_STRONG; } else if (optcmp(long_options[option_index].name, "send-ip") == 0) { Index: nmap.h

--- nmap.h (revision 13870) +++ nmap.h (working copy) @@ -399,6 +399,10 @@ #define MAXHOSTNAMELEN 64 #endif +/* Max payload: Worst case is IPv4 with 40bytes of options and TCP with 20 + * bytes of options. */ +#define MAX_PAYLOAD_ALLOWED 65535-60-40 + #ifndef recvfrom6_t # define recvfrom6_t int #endif Index: NmapOps.h

--- NmapOps.h (revision 13870) +++ NmapOps.h (working copy) @@ -245,7 +245,7 @@

int max_ips_to_scan; // Used for Random input (-iR) to specify how // many IPs to try before stopping. 0 means unlimited.

Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://SecLists.Org

Current thread: