From 988bb26828fc4f18a42b9eb44bf513a1eacf1066 Mon Sep 17 00:00:00 2001
From: Jeff Lucovsky <jeff@lucovsky.org>
Date: Wed, 4 Nov 2020 08:34:51 -0500
Subject: [PATCH] decode: Improved handling of ICMPv4 messages

This commit improves handling of ICMPv4 messages, especially those with
variable sized headers.

This commit also adds a header length variable for use by the new
sticky buffer for the header.
---
 src/decode-icmpv4.c | 32 +++++++++++++++++++++++++++++---
 src/decode-icmpv4.h | 22 ++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/src/decode-icmpv4.c b/src/decode-icmpv4.c
index e8748cac6f..90d67bab40 100644
--- a/src/decode-icmpv4.c
+++ b/src/decode-icmpv4.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2010 Open Information Security Foundation
+/* Copyright (C) 2007-2020 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -168,8 +168,6 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
     p->proto = IPPROTO_ICMP;
     p->icmp_s.type = p->icmpv4h->type;
     p->icmp_s.code = p->icmpv4h->code;
-    p->payload = (uint8_t *)pkt + ICMPV4_HEADER_LEN;
-    p->payload_len = len - ICMPV4_HEADER_LEN;
 
     int ctype = ICMPv4GetCounterpart(p->icmp_s.type);
     if (ctype != -1) {
@@ -177,6 +175,7 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
     }
 
     ICMPV4ExtHdr* icmp4eh = (ICMPV4ExtHdr*) p->icmpv4h;
+    p->icmpv4vars.hlen = ICMPV4_HEADER_LEN;
 
     switch (p->icmpv4h->type)
     {
@@ -270,6 +269,12 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
             if (p->icmpv4h->code!=0) {
                 ENGINE_SET_EVENT(p,ICMPV4_UNKNOWN_CODE);
             }
+
+            if (len < (sizeof(ICMPV4Timestamp) + ICMPV4_HEADER_LEN)) {
+                ENGINE_SET_EVENT(p, ICMPV4_IPV4_TRUNC_PKT);
+            } else {
+                p->icmpv4vars.hlen += sizeof(ICMPV4Timestamp);
+            }
             break;
 
         case ICMP_TIMESTAMPREPLY:
@@ -278,6 +283,12 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
             if (p->icmpv4h->code!=0) {
                 ENGINE_SET_EVENT(p,ICMPV4_UNKNOWN_CODE);
             }
+
+            if (len < (sizeof(ICMPV4Timestamp) + ICMPV4_HEADER_LEN)) {
+                ENGINE_SET_EVENT(p, ICMPV4_IPV4_TRUNC_PKT);
+            } else {
+                p->icmpv4vars.hlen += sizeof(ICMPV4Timestamp);
+            }
             break;
 
         case ICMP_INFO_REQUEST:
@@ -296,6 +307,18 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
             }
             break;
 
+        case ICMP_ROUTERADVERT: {
+            /* pkt points to beginning of icmp message */
+            ICMPV4RtrAdvert *icmpv4_router_advert = (ICMPV4RtrAdvert *)(pkt + sizeof(ICMPV4Hdr));
+            uint32_t advert_len = icmpv4_router_advert->naddr *
+                                  (icmpv4_router_advert->addr_sz * sizeof(uint32_t));
+            if (len < (advert_len + ICMPV4_HEADER_LEN)) {
+                ENGINE_SET_EVENT(p, ICMPV4_IPV4_TRUNC_PKT);
+            } else {
+                p->icmpv4vars.hlen += advert_len;
+            }
+        } break;
+
         case ICMP_ADDRESS:
             p->icmpv4vars.id=icmp4eh->id;
             p->icmpv4vars.seq=icmp4eh->seq;
@@ -317,6 +340,9 @@ int DecodeICMPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
 
     }
 
+    p->payload = (uint8_t *)pkt + p->icmpv4vars.hlen;
+    p->payload_len = len - p->icmpv4vars.hlen;
+
     FlowSetupPacket(p);
     return TM_ECODE_OK;
 }
diff --git a/src/decode-icmpv4.h b/src/decode-icmpv4.h
index 5f9946f0a3..8d44c17875 100644
--- a/src/decode-icmpv4.h
+++ b/src/decode-icmpv4.h
@@ -186,6 +186,9 @@ typedef struct ICMPV4Vars_
     uint16_t  id;
     uint16_t  seq;
 
+    /** Actual header length **/
+    uint32_t hlen;
+
     /** Pointers to the embedded packet headers */
     IPV4Hdr *emb_ipv4h;
     TCPHdr *emb_tcph;
@@ -203,6 +206,23 @@ typedef struct ICMPV4Vars_
     uint16_t emb_dport;
 } ICMPV4Vars;
 
+/* ICMPV4 Router Advertisement - fixed components */
+/* actual size determined by address count and size */
+typedef struct ICMPV4RtrAdvert_ {
+    /** Number of advertised addresses **/
+    uint8_t naddr;
+
+    /** Size of each advertised address **/
+    uint8_t addr_sz;
+} __attribute__((__packed__)) ICMPV4RtrAdvert;
+
+/* ICMPV4 TImestamp messages */
+typedef struct ICMPV4Timestamp_ {
+    uint32_t orig_ts;
+    uint32_t rx_ts;
+    uint32_t tx_ts;
+} __attribute__((__packed__)) ICMPV4Timestamp;
+
 #define CLEAR_ICMPV4_PACKET(p) do { \
     (p)->level4_comp_csum = -1;     \
     PACKET_CLEAR_L4VARS((p));       \
@@ -238,6 +258,8 @@ typedef struct ICMPV4Vars_
 #define ICMPV4_GET_EMB_UDP(p)      (p)->icmpv4vars.emb_udph
 /** macro for icmpv4 embedded "icmpv4h" header access */
 #define ICMPV4_GET_EMB_ICMPV4H(p)  (p)->icmpv4vars.emb_icmpv4h
+/** macro for icmpv4 header length */
+#define ICMPV4_GET_HLEN_ICMPV4H(p) (p)->icmpv4vars.hlen
 
 /** macro for checking if a ICMP DEST UNREACH packet is valid for use
  *  in other parts of the engine, such as the flow engine.