decode: limits the number of decoded layers

so as to avoid overrecursion leading to stack exhaustion

(cherry picked from commit 7500c29300dcef8716d87461842e7d7c3e5101ac)

diff --git a/rules/decoder-events.rules b/rules/decoder-events.rules
index 6518a17500de3e8305282babdd607d613474daf4..3dea158ac1f38418b110274ec7e40dc86f152de8 100644 (file)
--- a/rules/decoder-events.rules
+++ b/rules/decoder-events.rules
@@ -143,5 +143,6 @@ alert pkthdr any any -> any any (msg:"SURICATA ERSPAN too many vlan layers"; dec
 # Cisco Fabric Path/DCE
 alert pkthdr any any -> any any (msg:"SURICATA DCE packet too small"; decode-event:dce.pkt_too_small; classtype:protocol-command-decode; sid:2200110; rev:2;)
 
-# next sid is 2200114
+alert pkthdr any any -> any any (msg:"SURICATA packet with too many layers"; decode-event:too_many_layers; classtype:protocol-command-decode; sid:2200111; rev:1;)
+# next sid is 2200112
 

diff --git a/src/decode-erspan.c b/src/decode-erspan.c
index dd2515bb0099dd572120dae894cd0f2b3988c827..b47e47838d7aad54fa02841508cfac3e3484df00 100644 (file)
--- a/src/decode-erspan.c
+++ b/src/decode-erspan.c
@@ -79,6 +79,9 @@ int DecodeERSPANTypeII(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const u
         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-ethernet.c b/src/decode-ethernet.c
index ff5daead51ff3dc8888dc265cc026882638e56fc..de08202dce03e05c1b50f4afe7c5c25db72a91dd 100644 (file)
--- a/src/decode-ethernet.c
+++ b/src/decode-ethernet.c
@@ -52,6 +52,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 e8e206cefb25711154b4a5ace7e906dcbe3982b5..a900bc3b15890e1e9ce5ff2ef75dfe8eeb4c1ad7 100644 (file)
--- a/src/decode-events.c
+++ b/src/decode-events.c
@@ -185,7 +185,15 @@ const struct DecodeEvents_ DEvents[] = {
     { "decoder.erspan.too_many_vlan_layers", ERSPAN_TOO_MANY_VLAN_LAYERS, },
 
     /* Cisco Fabric Path/DCE events. */
-    { "decoder.dce.pkt_too_small", DCE_PKT_TOO_SMALL, },
+    {
+            "decoder.dce.pkt_too_small",
+            DCE_PKT_TOO_SMALL,
+    },
+
+    {
+            "decoder.too_many_layers",
+            GENERIC_TOO_MANY_LAYERS,
+    },
 
     /* STREAM EVENTS */
     { "stream.3whs_ack_in_wrong_dir", STREAM_3WHS_ACK_IN_WRONG_DIR, },

diff --git a/src/decode-events.h b/src/decode-events.h
index 347fb5c15462119686f7f6274cbd67cd417613b6..088453315ba99e721b42905d18674df0e4d88a1a 100644 (file)
--- a/src/decode-events.h
+++ b/src/decode-events.h
@@ -194,8 +194,11 @@ enum {
     /* Cisco Fabric Path/DCE events. */
     DCE_PKT_TOO_SMALL,
 
+    /* generic events */
+    GENERIC_TOO_MANY_LAYERS,
+
     /* END OF DECODE EVENTS ON SINGLE PACKET */
-    DECODE_EVENT_PACKET_MAX = DCE_PKT_TOO_SMALL,
+    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 7457dda7eae093fc4b81083c25c6346852404737..8179af19c586eaa636691e847fe02b7f8cd96cd0 100644 (file)
--- a/src/decode-geneve.c
+++ b/src/decode-geneve.c
@@ -192,6 +192,9 @@ int DecodeGeneve(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     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 01e2553edc623b77fdd7ece10693bf38119b8f42..c52649e9cad59f489fe68aa3489ab83d16ef464c 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 478c6a66b4aeb0fb530fc1b39983410798b2f229..17246f72512b08985cafc97bb8eb059e7b5370af 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 6b6701fd445148826d2327acb983e4eaa11ca1c5..8ddce2e10a1e771716772909d945f9047f1d7570 100644 (file)
--- a/src/decode-ipv6.c
+++ b/src/decode-ipv6.c
@@ -586,6 +586,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 de23b8dc71c17cb8350687a18e36e63fbda1db3d..5f589979f84c65cc3da31fa7e1f221e334c2ab20 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-ppp.c b/src/decode-ppp.c
index bc12904d8a1118c99acbd2ba82472805e7103866..7e3668047af84c75cc50e7a475a220915cdc19a7 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 41051fe4fd02b150ca604fa94810fa75033f9301..43f5c2d04f447d82c6964dd4be492ae7a23ed748 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 f2331066956e989a0e5a3c41f20e858fee4f0d26..a282a39a6387d89cefa07955b09e6baa4bc61ba7 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 979014bc9b67fa575e78c164ed79423037e5d1b2..5d1f2c2757eeb783e06050ccea5dc117cc1d2e50 100644 (file)
--- a/src/decode-vlan.c
+++ b/src/decode-vlan.c
@@ -73,6 +73,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 746d9b979224ff8ae9bcac5e2d7d1f1f8f4daafc..4c2c2a623d20cbce7a6aed6a055983c7889a8061 100644 (file)
--- a/src/decode-vxlan.c
+++ b/src/decode-vxlan.c
@@ -124,6 +124,9 @@ int DecodeVXLAN(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     if (len < (sizeof(VXLANHeader) + 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 22ad5e52f1f02df1f7c6ca9e473f3181fbef0c48..0aad1672be3fc37b2dcf1e4e05b33941e92ea14a 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, PacketQueue *pq, enum DecodeTunnelProto proto)
+static int DecodeTunnel(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t,
+        PacketQueue *, enum DecodeTunnelProto) WARN_UNUSED;
+
+static int DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *pkt,
+        uint32_t len, PacketQueue *pq, enum DecodeTunnelProto proto)
 {
     switch (proto) {
         case DECODE_TUNNEL_PPP:
@@ -739,6 +743,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 bda7a6e03f2902251260a5e16f49609de4604880..01a259637dfca4af7151a2502f60efa309b4d00c 100644 (file)
--- a/src/decode.h
+++ b/src/decode.h
@@ -601,6 +601,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
@@ -813,6 +818,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 { \
@@ -933,7 +939,6 @@ int DecodeSll(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint3
 int DecodePPP(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *);
 int DecodePPPOESession(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *);
 int DecodePPPOEDiscovery(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *);
-int DecodeTunnel(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *, enum DecodeTunnelProto) __attribute__ ((warn_unused_result));
 int DecodeNull(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *);
 int DecodeRaw(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint32_t, PacketQueue *);
 int DecodeIPV4(ThreadVars *, DecodeThreadVars *, Packet *, const uint8_t *, uint16_t, PacketQueue *);
@@ -1142,6 +1147,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 9c10b6939f869ad80a549a98c21ff43bd5e73b63..3ce79d533749431129be798fb7842b71ba301073 100644 (file)
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -1357,6 +1357,9 @@ decoder:
     enabled: false
     ports: $GENEVE_PORTS # syntax: '[6081, 1234]' or '6081'.
 
+  # maximum number of decoder layers for a packet
+  # max-layers: 16
+
 ##
 ## Performance tuning and profiling
 ##