ipv4: fail packet decoding on bad ipv4 option length

Currently all failures in IPv4 option decode are ignore with
respect to continuing to handle the packet.

Change this to fail, and abort handling the packet if the
option length is invalid.

Ticket 3328:
https://redmine.openinfosecfoundation.org/issues/3328

diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c
index 0832b9de5672e1ff16d9180cf49802ab31f5a3e5..9c0a216f2fa22f54942cdcfd6531bf06f7dfbe7b 100644 (file)
--- a/src/decode-ipv4.c
+++ b/src/decode-ipv4.c
@@ -299,7 +299,7 @@ typedef struct IPV4Options_ {
 /**
  * Decode/Validate IPv4 Options.
  */
-static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4Options *opts)
+static int DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4Options *opts)
 {
     uint16_t plen = len;
 
@@ -353,7 +353,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
             /* Option length is too big for packet */
             if (unlikely(*(pkt+1) > plen)) {
                 ENGINE_SET_INVALID_EVENT(p, IPV4_OPT_INVALID_LEN);
-                return;
+                return -1;
             }
 
             IPV4Opt opt = {*pkt, *(pkt+1), plen > 2 ? (pkt + 2) : NULL };
@@ -363,7 +363,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
              * Also check for invalid lengths 0 and 1. */
             if (unlikely(opt.len > plen || opt.len < 2)) {
                 ENGINE_SET_INVALID_EVENT(p, IPV4_OPT_INVALID_LEN);
-                return;
+                return -1;
             }
             /* we are parsing the most commonly used opts to prevent
              * us from having to walk the opts list for these all the
@@ -376,7 +376,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateTimestamp(p, &opt)) {
-                        return;
+                        return 0;
                     }
                     opts->o_ts = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_TS;
@@ -387,7 +387,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateRoute(p, &opt) != 0) {
-                        return;
+                        return 0;
                     }
                     opts->o_rr = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_RR;
@@ -398,7 +398,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateGeneric(p, &opt)) {
-                        return;
+                        return 0;
                     }
                     opts->o_qs = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_QS;
@@ -409,7 +409,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateGeneric(p, &opt)) {
-                        return;
+                        return 0;
                     }
                     opts->o_sec = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_SEC;
@@ -420,7 +420,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateRoute(p, &opt) != 0) {
-                        return;
+                        return 0;
                     }
                     opts->o_lsrr = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_LSRR;
@@ -431,7 +431,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateCIPSO(p, &opt) != 0) {
-                        return;
+                        return 0;
                     }
                     opts->o_cipso = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_CIPSO;
@@ -442,7 +442,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateGeneric(p, &opt)) {
-                        return;
+                        return 0;
                     }
                     opts->o_sid = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_SID;
@@ -453,7 +453,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateRoute(p, &opt) != 0) {
-                        return;
+                        return 0;
                     }
                     opts->o_ssrr = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_SSRR;
@@ -464,7 +464,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
                         /* Warn - we can keep going */
                         break;
                     } else if (IPV4OptValidateGeneric(p, &opt)) {
-                        return;
+                        return 0;
                     }
                     opts->o_rtralt = opt;
                     p->ip4vars.opts_set |= IPV4_OPT_FLAG_RTRALT;
@@ -482,6 +482,7 @@ static void DecodeIPV4Options(Packet *p, const uint8_t *pkt, uint16_t len, IPV4O
         }
     }
 
+    return 0;
 }
 
 static int DecodeIPV4Packet(Packet *p, const uint8_t *pkt, uint16_t len)
@@ -523,7 +524,9 @@ static int DecodeIPV4Packet(Packet *p, const uint8_t *pkt, uint16_t len)
     if (ip_opt_len > 0) {
         IPV4Options opts;
         memset(&opts, 0x00, sizeof(opts));
-        DecodeIPV4Options(p, pkt + IPV4_HEADER_LEN, ip_opt_len, &opts);
+        if (DecodeIPV4Options(p, pkt + IPV4_HEADER_LEN, ip_opt_len, &opts) < 0) {
+            return -1;
+        }
     }
 
     return 0;
@@ -708,7 +711,7 @@ static int DecodeIPV4OptionsRRTest02(void)
 
     IPV4Options opts;
     memset(&opts, 0x00, sizeof(opts));
-    DecodeIPV4Options(p, raw_opts, sizeof(raw_opts), &opts);
+    FAIL_IF(DecodeIPV4Options(p, raw_opts, sizeof(raw_opts), &opts) != -1);
     FAIL_IF((p->flags & PKT_IS_INVALID) == 0);
     FAIL_IF(opts.o_rr.type != 0);
     SCFree(p);