From: Jason Ish <ish@unx.ca>
Date: Mon, 5 Sep 2016 13:41:33 +0000 (-0600)
Subject: icmpv6: fix checksum verification if fcs present
X-Git-Tag: suricata-3.1.2~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=af4085b77b56cbb41496956e5b23184b4528a094;p=thirdparty%2Fsuricata.git

icmpv6: fix checksum verification if fcs present

Calculate the length of the ICMPv6 packet from decoded information
instead of off the wire length. This will provide the correct
length if trailing data like an FCS is present.

Fixes issue:
https://redmine.openinfosecfoundation.org/issues/1849
---

diff --git a/src/decode-icmpv6.c b/src/decode-icmpv6.c
index 26a48b7439..75da935bfa 100644
--- a/src/decode-icmpv6.c
+++ b/src/decode-icmpv6.c
@@ -1603,6 +1603,61 @@ end:
     return retval;
 }
 
+/**
+ * \test Test for valid ICMPv6 checksum when the FCS is still attached.
+ *
+ * Tests that the packet is decoded with sufficient info to verify the
+ * checksum even if the packet has some trailing data like an ethernet
+ * FCS.
+ */
+static int ICMPV6CalculateValidChecksumWithFCS(void)
+{
+    /* IPV6/ICMPv6 packet with ethernet header.
+     * - IPv6 payload length: 36
+     */
+    uint8_t raw_ipv6[] = {
+        0x33, 0x33, 0x00, 0x00, 0x00, 0x16, 0x00, 0x50,
+        0x56, 0xa6, 0x6a, 0x7d, 0x86, 0xdd, 0x60, 0x00,
+        0x00, 0x00, 0x00, 0x24, 0x00, 0x01, 0xfe, 0x80,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf5, 0x09,
+        0xad, 0x44, 0x49, 0x38, 0x5f, 0xa9, 0xff, 0x02,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x16, 0x3a, 0x00,
+        0x05, 0x02, 0x00, 0x00, 0x01, 0x00, 0x8f, 0x00,
+        0x24, 0xe0, 0x00, 0x00, 0x00, 0x01, 0x03, 0x00, /* Checksum: 0x24e0. */
+        0x00, 0x00, 0xff, 0x02, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0xfb, 0x1f, 0x34, 0xf6, 0xa4
+    };
+    uint16_t csum = *(((uint16_t *)(raw_ipv6 + 64)));
+
+    Packet *p = SCMalloc(SIZE_OF_PACKET);
+    FAIL_IF_NULL(p);
+    IPV6Hdr ip6h;
+    ThreadVars tv;
+    DecodeThreadVars dtv;
+
+    memset(&tv, 0, sizeof(ThreadVars));
+    memset(p, 0, SIZE_OF_PACKET);
+    memset(&dtv, 0, sizeof(DecodeThreadVars));
+    memset(&ip6h, 0, sizeof(IPV6Hdr));
+
+    FlowInitConfig(FLOW_QUIET);
+    DecodeIPV6(&tv, &dtv, p, raw_ipv6 + 14, sizeof(raw_ipv6) - 14, NULL);
+    FAIL_IF_NULL(p->icmpv6h);
+
+    uint16_t icmpv6_len = IPV6_GET_RAW_PLEN(p->ip6h) -
+        ((uint8_t *)p->icmpv6h - (uint8_t *)p->ip6h - IPV6_HEADER_LEN);
+    FAIL_IF(icmpv6_len != 28);
+    FAIL_IF(ICMPV6CalculateChecksum(p->ip6h->s_ip6_addrs,
+            (uint16_t *)p->icmpv6h, icmpv6_len) != csum);
+
+    PACKET_RECYCLE(p);
+    FlowShutdown();
+    SCFree(p);
+    PASS;
+}
+
 #endif /* UNITTESTS */
 /**
  * \brief Registers ICMPV6 unit tests
@@ -1654,6 +1709,8 @@ void DecodeICMPV6RegisterTests(void)
     UtRegisterTest("ICMPV6RedirectTestKnownCode", ICMPV6RedirectTestKnownCode);
     UtRegisterTest("ICMPV6RedirectTestUnknownCode",
                    ICMPV6RedirectTestUnknownCode);
+    UtRegisterTest("ICMPV6CalculateValidChecksumWithFCS",
+                   ICMPV6CalculateValidChecksumWithFCS);
 #endif /* UNITTESTS */
 }
 /**
diff --git a/src/detect-csum.c b/src/detect-csum.c
index 37d2b22163..4a338f5409 100644
--- a/src/detect-csum.c
+++ b/src/detect-csum.c
@@ -814,10 +814,13 @@ int DetectICMPV6CsumMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx,
         return cd->valid;
     }
 
-    if (p->level4_comp_csum == -1)
+    if (p->level4_comp_csum == -1) {
+        uint16_t len = IPV6_GET_RAW_PLEN(p->ip6h) -
+            ((uint8_t *)p->icmpv6h - (uint8_t *)p->ip6h - IPV6_HEADER_LEN);
         p->level4_comp_csum = ICMPV6CalculateChecksum(p->ip6h->s_ip6_addrs,
                                                       (uint16_t *)p->icmpv6h,
-                                                      GET_PKT_LEN(p) - ((uint8_t *)p->icmpv6h - GET_PKT_DATA(p)));
+                                                      len);
+    }
 
     if (p->level4_comp_csum == p->icmpv6h->csum && cd->valid == 1)
         return 1;