From 5d726c7733c9abc655ea574f4fe2fe4db46ae0d3 Mon Sep 17 00:00:00 2001
From: dmiller <dmiller@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Thu, 28 Jul 2016 05:11:35 +0000
Subject: [PATCH] Handle ICMPv6 packets without a body

Two issues here: First, IP protocol scan can send packets with protocol
58 (ICMPv6) even over IPv4. This led to a bad interaction where the
packet was created (in build_protoscan_packet) without a data payload,
but setIP tried to set the packet's Identifier field (present in both
ICMPv6 and ICMP Echo Request packets), leading to a heap buffer
overflow. Instead, we now only try to set this identifier when the IP
version matches the ICMP version, indicating that we set the data
payload.

The other issue was a out-of-bounds read while packet tracing when an
ICMPv6 packet without a payload was sent or received, due to trying to
read the type and code. Now we check that the data length is sufficient
to contain an ICMPv6 header before attempting to read one.

Credit LLVM/Clang's AddressSanitizer with catching these bugs.
---
 libnetutil/netutil.cc | 18 ++++++++++++------
 scan_engine_raw.cc    |  9 +++++----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/libnetutil/netutil.cc b/libnetutil/netutil.cc
index 8ab28dc61..d5af1d284 100644
--- a/libnetutil/netutil.cc
+++ b/libnetutil/netutil.cc
@@ -3024,15 +3024,21 @@ icmpbad:
         srchost, dsthost, icmptype, icmpinfo, icmpfields, ipinfo);
     }
 
-    /* UNKNOWN PROTOCOL **********************************************************/
   } else if (hdr.proto == IPPROTO_ICMPV6) {
-    const struct icmpv6_hdr *icmpv6;
+    if (datalen > sizeof(struct icmpv6_hdr)) {
+      const struct icmpv6_hdr *icmpv6;
 
-    icmpv6 = (struct icmpv6_hdr *) data;
-    Snprintf(protoinfo, sizeof(protoinfo), "ICMPv6 (%d) %s > %s (type=%d/code=%d) %s",
-      hdr.proto, srchost, dsthost,
-      icmpv6->icmpv6_type, icmpv6->icmpv6_code, ipinfo);
+      icmpv6 = (struct icmpv6_hdr *) data;
+      Snprintf(protoinfo, sizeof(protoinfo), "ICMPv6 (%d) %s > %s (type=%d/code=%d) %s",
+          hdr.proto, srchost, dsthost,
+          icmpv6->icmpv6_type, icmpv6->icmpv6_code, ipinfo);
+    }
+    else {
+      Snprintf(protoinfo, sizeof(protoinfo), "ICMPv6 (%d) %s > %s (type=?/code=?) %s",
+          hdr.proto, srchost, dsthost, ipinfo);
+    }
   } else {
+    /* UNKNOWN PROTOCOL **********************************************************/
     const char *hdrstr;
 
     hdrstr = nexthdrtoa(hdr.proto, 1);
diff --git a/scan_engine_raw.cc b/scan_engine_raw.cc
index 11c439302..e3bfdf255 100644
--- a/scan_engine_raw.cc
+++ b/scan_engine_raw.cc
@@ -181,20 +181,21 @@ void UltraProbe::setIP(u8 *ippacket, u32 len, const probespec *pspec) {
   }
 
   if (hdr == IPPROTO_TCP) {
-    assert(len >= 20);
+    assert(len >= sizeof(struct tcp_hdr));
     tcp = (struct tcp_hdr *) data;
     probes.IP.pd.tcp.sport = ntohs(tcp->th_sport);
     probes.IP.pd.tcp.seq = ntohl(tcp->th_seq);
   } else if (hdr == IPPROTO_UDP) {
-    assert(len >= 8);
+    assert(len >= sizeof(struct udp_hdr));
     udp = (struct udp_hdr *) data;
     probes.IP.pd.udp.sport = ntohs(udp->uh_sport);
   } else if (hdr == IPPROTO_SCTP) {
-    assert(len >= 12);
+    assert(len >= sizeof(struct sctp_hdr));
     sctp = (struct sctp_hdr *) data;
     probes.IP.pd.sctp.sport = ntohs(sctp->sh_sport);
     probes.IP.pd.sctp.vtag = ntohl(sctp->sh_vtag);
-  } else if (hdr == IPPROTO_ICMP || hdr == IPPROTO_ICMPV6) {
+  } else if ((ip->ip_v == 4 && hdr == IPPROTO_ICMP) || (ip->ip_v == 6 && hdr == IPPROTO_ICMPV6)) {
+    assert(len >= sizeof(struct ppkt));
     icmp = (struct ppkt *) data;
     probes.IP.pd.icmp.ident = ntohs(icmp->id);
   }