decode: limits the number of decoded layers

so as to avoid overrecursion leading to stack exhaustion

diff --git a/rules/decoder-events.rules b/rules/decoder-events.rules
index d5e001f9c71299c66e2343e45f3e5c2fe73b04e7..661fb13a1e667afd52e79ba1f795f1c5d067cfe9 100644 (file)
--- a/rules/decoder-events.rules
+++ b/rules/decoder-events.rules
@@ -146,5 +146,7 @@ alert pkthdr any any -> any any (msg:"SURICATA DCE packet too small"; decode-eve
 # Cisco HDLC
 alert pkthdr any any -> any any (msg:"SURICATA CHDLC packet too small"; decode-event:chdlc.pkt_too_small; classtype:protocol-command-decode; sid:2200115; rev:1;)
 
-# next sid is 2200116
+alert pkthdr any any -> any any (msg:"SURICATA packet with too many layers"; decode-event:too_many_layers; classtype:protocol-command-decode; sid:2200116; rev:1;)
+
+# next sid is 2200117
 

diff --git a/src/decode-chdlc.c b/src/decode-chdlc.c
index a7ed61869d02a090478f4ac09b59b54e33319074..4607eb0a1c8753a84eb6957aca62bff374b0b71b 100644 (file)
--- a/src/decode-chdlc.c
+++ b/src/decode-chdlc.c
@@ -51,6 +51,9 @@ int DecodeCHDLC(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
     if (unlikely(len > CHDLC_HEADER_LEN + USHRT_MAX)) {
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     CHDLCHdr *hdr = (CHDLCHdr *)pkt;
     if (unlikely(hdr == NULL))

diff --git a/src/decode-erspan.c b/src/decode-erspan.c
index c70b1a32aa160ce40b2c538347cef5a1925753c0..ab326a0d626f1e3490bf541ea3f085b3e3366c5f 100644 (file)
--- a/src/decode-erspan.c
+++ b/src/decode-erspan.c
@@ -80,6 +80,9 @@ int DecodeERSPAN(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
         ENGINE_SET_EVENT(p,ERSPAN_HEADER_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     const ErspanHdr *ehdr = (const ErspanHdr *)pkt;
     uint16_t version = SCNtohs(ehdr->ver_vlan) >> 12;

diff --git a/src/decode-esp.c b/src/decode-esp.c
index 575035e793519ed30f6aec1ade4d1ddb79610e80..205767967512988801ec5dc79aab22996bb942f6 100644 (file)
--- a/src/decode-esp.c
+++ b/src/decode-esp.c
@@ -61,6 +61,9 @@ int DecodeESP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *p
 {
     StatsIncr(tv, dtv->counter_esp);
 
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     if (unlikely(DecodeESPPacket(tv, p, pkt, len) < 0)) {
         CLEAR_ESP_PACKET(p);
         return TM_ECODE_FAILED;

diff --git a/src/decode-ethernet.c b/src/decode-ethernet.c
index e46c09cd5f2c00304982928359f143bd5fdd7bfa..556f5ed338882f4b72a9b94de4ddd753de0333ed 100644 (file)
--- a/src/decode-ethernet.c
+++ b/src/decode-ethernet.c
@@ -48,6 +48,9 @@ int DecodeEthernet(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         return TM_ECODE_FAILED;
     }
 
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     p->ethh = (EthernetHdr *)pkt;
     if (unlikely(p->ethh == NULL))
         return TM_ECODE_FAILED;

diff --git a/src/decode-events.c b/src/decode-events.c
index ee8fa24b75e3243e4b9be232a4c34d0fe7f69ac3..286d9a902f36ae7894bd7841cad90625ae71f415 100644 (file)
--- a/src/decode-events.c
+++ b/src/decode-events.c
@@ -566,6 +566,10 @@ const struct DecodeEvents_ DEvents[] = {
             "decoder.nsh.unknown_payload",
             NSH_UNKNOWN_PAYLOAD,
     },
+    {
+            "decoder.too_many_layers",
+            GENERIC_TOO_MANY_LAYERS,
+    },
 
     /* STREAM EVENTS */
     {

diff --git a/src/decode-events.h b/src/decode-events.h
index c34ae80ac4b7cbd47d9ee3c861f5f15f99456aa6..31fedc7e2d15b99c048811e2cb545bca57ebbb92 100644 (file)
--- a/src/decode-events.h
+++ b/src/decode-events.h
@@ -211,8 +211,11 @@ enum {
     NSH_UNSUPPORTED_TYPE,
     NSH_UNKNOWN_PAYLOAD,
 
+    /* generic events */
+    GENERIC_TOO_MANY_LAYERS,
+
     /* END OF DECODE EVENTS ON SINGLE PACKET */
-    DECODE_EVENT_PACKET_MAX = NSH_UNKNOWN_PAYLOAD,
+    DECODE_EVENT_PACKET_MAX = GENERIC_TOO_MANY_LAYERS,
 
     /* STREAM EVENTS */
     STREAM_3WHS_ACK_IN_WRONG_DIR,

diff --git a/src/decode-geneve.c b/src/decode-geneve.c
index 41ed3ebcf7c2f92c8feb9bd998cf69e922f79385..88c6ade8259dad743c713f67cba5dab29da9a7fe 100644 (file)
--- a/src/decode-geneve.c
+++ b/src/decode-geneve.c
@@ -194,6 +194,9 @@ int DecodeGeneve(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t
 
     if (unlikely(len < GENEVE_MIN_HEADER_LEN))
         return TM_ECODE_FAILED;
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     /* Specific Geneve header field validation */
     geneve_hdr_len = GENEVE_TOTAL_HEADER_LEN(geneve_hdr);

diff --git a/src/decode-gre.c b/src/decode-gre.c
index a55352a7339b966e3b7d26d2f88b607560ed9849..078b9bfae5228b30af57ba10e50570894e805932 100644 (file)
--- a/src/decode-gre.c
+++ b/src/decode-gre.c
@@ -54,6 +54,9 @@ int DecodeGRE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *p
         ENGINE_SET_INVALID_EVENT(p, GRE_PKT_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     p->greh = (GREHdr *)pkt;
     if(p->greh == NULL)

diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c
index 60bb9e63d9508f7c21127c22a270cd7d4a2a3d84..171b7d636e5a8b6e5fcffbfdf92f7020b9cf27e8 100644 (file)
--- a/src/decode-ipv4.c
+++ b/src/decode-ipv4.c
@@ -521,6 +521,9 @@ int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     SCLogDebug("pkt %p len %"PRIu16"", pkt, len);
 
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     /* do the actual decoding */
     if (unlikely(DecodeIPV4Packet (p, pkt, len) < 0)) {
         SCLogDebug("decoding IPv4 packet failed");

diff --git a/src/decode-ipv6.c b/src/decode-ipv6.c
index 05ef92d1bde92af9d49d36bf63d0209e414da766..22e9b5d7d0f1af2918e6d0e04569c35d07c1f263 100644 (file)
--- a/src/decode-ipv6.c
+++ b/src/decode-ipv6.c
@@ -565,6 +565,9 @@ int DecodeIPV6(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *
 {
     StatsIncr(tv, dtv->counter_ipv6);
 
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     /* do the actual decoding */
     int ret = DecodeIPV6Packet (tv, dtv, p, pkt, len);
     if (unlikely(ret < 0)) {

diff --git a/src/decode-mpls.c b/src/decode-mpls.c
index e64df63b7f77c55b41138ae06f1cdfba856fc7bc..6de6e49e2d08ccf8b9addff8d0522b9b5b339f61 100644 (file)
--- a/src/decode-mpls.c
+++ b/src/decode-mpls.c
@@ -53,6 +53,9 @@ int DecodeMPLS(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     StatsIncr(tv, dtv->counter_mpls);
 
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     do {
         if (len < MPLS_HEADER_LEN) {
             ENGINE_SET_INVALID_EVENT(p, MPLS_HEADER_TOO_SMALL);

diff --git a/src/decode-nsh.c b/src/decode-nsh.c
index 7a3355ddb19e012628183ff13e0b038611d697f2..f3dd542ee6094e0a3af4fc50255013455ed40fff 100644 (file)
--- a/src/decode-nsh.c
+++ b/src/decode-nsh.c
@@ -51,6 +51,9 @@ int DecodeNSH(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *p
         ENGINE_SET_INVALID_EVENT(p, NSH_HEADER_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     /* Sanity check the header version */
     const NshHdr *hdr = (const NshHdr *)pkt;

diff --git a/src/decode-ppp.c b/src/decode-ppp.c
index 7a865c5d487bca9ecf4cc8f583f987a3d13efb0c..7cb311c4f5322a156065fd34fa11d8a3f24a3b3e 100644 (file)
--- a/src/decode-ppp.c
+++ b/src/decode-ppp.c
@@ -49,6 +49,9 @@ int DecodePPP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         ENGINE_SET_INVALID_EVENT(p, PPP_PKT_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     p->ppph = (PPPHdr *)pkt;
     if (unlikely(p->ppph == NULL))

diff --git a/src/decode-sll.c b/src/decode-sll.c
index 9e02b1c8c5af482cd0af4b7c695b4ff7488973e5..7bfe5799d2245c6863aa21277ee1532b10757e48 100644 (file)
--- a/src/decode-sll.c
+++ b/src/decode-sll.c
@@ -45,6 +45,9 @@ int DecodeSll(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         ENGINE_SET_INVALID_EVENT(p, SLL_PKT_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     SllHdr *sllh = (SllHdr *)pkt;
     if (unlikely(sllh == NULL))

diff --git a/src/decode-template.c b/src/decode-template.c
index f47c736929b3bb273386458be4d903ece838d6db..15091df78aaa860c19ab86a217f8284ffb38f00d 100644 (file)
--- a/src/decode-template.c
+++ b/src/decode-template.c
@@ -62,6 +62,15 @@ int DecodeTEMPLATE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         //ENGINE_SET_EVENT(p,TEMPLATE_HEADER_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    /* Each packet keeps a count of decoded layers
+     * This function increases it and returns false
+     * if we have too many decoded layers, such as
+     * ethernet/MPLS/ethernet/MPLS... which may
+     * lead to stack overflow by a too deep recursion
+     */
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     /* Now we can access the header */
     const TemplateHdr *hdr = (const TemplateHdr *)pkt;

diff --git a/src/decode-vlan.c b/src/decode-vlan.c
index d929bae826a8d098ced1cbd3655a0e05e38bca8a..aba0ec93fc1c381f83c069ed42f4d06d1961dea0 100644 (file)
--- a/src/decode-vlan.c
+++ b/src/decode-vlan.c
@@ -70,6 +70,9 @@ int DecodeVLAN(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         ENGINE_SET_INVALID_EVENT(p, VLAN_HEADER_TOO_SMALL);
         return TM_ECODE_FAILED;
     }
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
     if (p->vlan_idx >= 2) {
         ENGINE_SET_EVENT(p,VLAN_HEADER_TOO_MANY_LAYERS);
         return TM_ECODE_FAILED;

diff --git a/src/decode-vxlan.c b/src/decode-vxlan.c
index dc952a8a12a3a7f63087326fbfdfd4a6e8376980..f5d754de0951208646bb3e34fc8c2789a9fa80c4 100644 (file)
--- a/src/decode-vxlan.c
+++ b/src/decode-vxlan.c
@@ -136,6 +136,9 @@ int DecodeVXLAN(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     if (len < (VXLAN_HEADER_LEN + sizeof(EthernetHdr)))
         return TM_ECODE_FAILED;
+    if (!PacketIncreaseCheckLayers(p)) {
+        return TM_ECODE_FAILED;
+    }
 
     const VXLANHeader *vxlanh = (const VXLANHeader *)pkt;
     if ((vxlanh->flags[0] & 0x08) == 0 || vxlanh->res != 0)

diff --git a/src/decode.c b/src/decode.c
index 384a7d94c2b73e1a09f29f475e7bb80acb12dc7b..9e4c33b41ebcf06c70fd6d79044e5985dd7dcad3 100644 (file)
--- a/src/decode.c
+++ b/src/decode.c
@@ -72,9 +72,13 @@ uint32_t default_packet_size = 0;
 extern bool stats_decoder_events;
 extern const char *stats_decoder_events_prefix;
 extern bool stats_stream_events;
+uint8_t decoder_max_layers = PKT_DEFAULT_MAX_DECODED_LAYERS;
 
-int DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
-        const uint8_t *pkt, uint32_t len, enum DecodeTunnelProto proto)
+static int DecodeTunnel(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t,
+        enum DecodeTunnelProto) WARN_UNUSED;
+
+static int DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *pkt,
+        uint32_t len, enum DecodeTunnelProto proto)
 {
     switch (proto) {
         case DECODE_TUNNEL_PPP:
@@ -764,6 +768,14 @@ void DecodeGlobalConfig(void)
     DecodeGeneveConfig();
     DecodeVXLANConfig();
     DecodeERSPANConfig();
+    intmax_t value = 0;
+    if (ConfGetInt("decoder.max-layers", &value) == 1) {
+        if (value < 0 || value > UINT8_MAX) {
+            SCLogWarning(SC_ERR_INVALID_VALUE, "Invalid value for decoder.max-layers");
+        } else {
+            decoder_max_layers = value;
+        }
+    }
 }
 
 /**

diff --git a/src/decode.h b/src/decode.h
index ea2613ecb72fb335d3ddf65ceb5bd2062e2400e3..d5d7d827f7c6095c7a6ab0b4ff8db60473b04ea2 100644 (file)
--- a/src/decode.h
+++ b/src/decode.h
@@ -610,6 +610,11 @@ typedef struct Packet_
      */
     struct PktPool_ *pool;
 
+    /* count decoded layers of packet : too many layers
+     * cause issues with performance and stability (stack exhaustion)
+     */
+    uint8_t nb_decoded_layers;
+
 #ifdef PROFILING
     PktProfiling *profile;
 #endif
@@ -831,6 +836,7 @@ void CaptureStatsSetup(ThreadVars *tv, CaptureStats *s);
         PACKET_RESET_CHECKSUMS((p));                                                               \
         PACKET_PROFILING_RESET((p));                                                               \
         p->tenant_id = 0;                                                                          \
+        p->nb_decoded_layers = 0;                                                                  \
     } while (0)
 
 #define PACKET_RECYCLE(p) do { \
@@ -952,7 +958,6 @@ int DecodeSll(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint3
 int DecodePPP(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t);
 int DecodePPPOESession(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t);
 int DecodePPPOEDiscovery(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t);
-int DecodeTunnel(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, enum DecodeTunnelProto) WARN_UNUSED;
 int DecodeNull(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t);
 int DecodeRaw(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t);
 int DecodeIPV4(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint16_t);
@@ -1161,6 +1166,19 @@ void DecodeUnregisterCounters(void);
 
 #define PKT_SET_SRC(p, src_val) ((p)->pkt_src = src_val)
 
+#define PKT_DEFAULT_MAX_DECODED_LAYERS 16
+extern uint8_t decoder_max_layers;
+
+static inline bool PacketIncreaseCheckLayers(Packet *p)
+{
+    p->nb_decoded_layers++;
+    if (p->nb_decoded_layers >= decoder_max_layers) {
+        ENGINE_SET_INVALID_EVENT(p, GENERIC_TOO_MANY_LAYERS);
+        return false;
+    }
+    return true;
+}
+
 /** \brief return true if *this* packet needs to trigger a verdict.
  *
  *  If we have the root packet, and we have none outstanding,

diff --git a/suricata.yaml.in b/suricata.yaml.in
index c1e6c207f541e8fcf9735ea0641baaca07f1866b..728d3e340f8e239bd655f2b13983f35f9eda81d8 100644 (file)
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -1361,6 +1361,9 @@ decoder:
     enabled: true
     ports: $GENEVE_PORTS # syntax: '[6081, 1234]' or '6081'.
 
+  # maximum number of decoder layers for a packet
+  # max-layers: 16
+
 ##
 ## Performance tuning and profiling
 ##