From df8db1ddb0736300bad4a7fee811d333ab77cb54 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 14 Nov 2019 11:34:56 -0600 Subject: [PATCH] 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 --- src/decode-ipv4.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c index 0832b9de56..9c0a216f2f 100644 --- 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);