From 02c9cf42f7d507337ecae2c853e33e33543055fd Mon Sep 17 00:00:00 2001 From: david Date: Sat, 29 Aug 2009 01:45:28 +0000 Subject: [PATCH] Fix some pointer arithmetic in build_icmp_raw. There were two bugs. The first is a pointer was kept to the beginning of the packet payload, and it was increased based on the varying size of the ICMP header. But its type was pointer to u32 instead of pointer to u8, so the expression datastart += 12 actually increased the pointer by 48 bytes, leaving garbage in the first 36 bytes of the payload and making it possible for the buffer to overflow. The second was that the remaining space left in the buffer was not decreased when the datastart was increased, again making it possible to overflow. I got a reliable segmentation fault with the command nmap -PP 1.2.3.4 --data-length 1480 --- tcpip.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tcpip.cc b/tcpip.cc index 235d62841..09f9cd5ae 100644 --- a/tcpip.cc +++ b/tcpip.cc @@ -1493,7 +1493,9 @@ struct ppkt { u16 seq; u8 data[1500]; /* Note -- first 4-12 bytes can be used for ICMP header */ } pingpkt; -u32 *datastart = (u32 *) pingpkt.data; +u8 *datastart = pingpkt.data; +/* dlen is the amount of space remaining in the data buffer; it may be reduced + depending on type. */ int dlen = sizeof(pingpkt.data); int icmplen=0; char *ping = (char *) &pingpkt; @@ -1507,11 +1509,11 @@ char *ping = (char *) &pingpkt; icmplen = 20; memset(datastart, 0, 12); datastart += 12; - //datalen -= 12; + dlen -= 12; } else if (ptype == 17 && pcode == 0) /* icmp netmask req */ { icmplen = 12; *datastart++ = 0; - //datalen -= 4; + dlen -= 4; } else fatal("Unknown icmp type/code (%d/%d) in %s", ptype, pcode, __func__);