From 8c19e5ff63757efa2a6874f749f062754a47c8b6 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 24 Jul 2014 16:50:34 +0200 Subject: [PATCH] ipv6: make exthdr parsing more robust Improve data length checks. Detect PadN option with 0 length. --- rules/decoder-events.rules | 4 +++- src/decode-events.h | 1 + src/decode-ipv6.c | 42 +++++++++++++++++++++++++++++++++++--- src/detect-engine-event.h | 1 + 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/rules/decoder-events.rules b/rules/decoder-events.rules index 9a3fb674eb..e711c21e73 100644 --- a/rules/decoder-events.rules +++ b/rules/decoder-events.rules @@ -37,6 +37,8 @@ alert pkthdr any any -> any any (msg:"SURICATA IPv6 DSTOPTS unknown option"; dec alert pkthdr any any -> any any (msg:"SURICATA IPv6 DSTOPTS only padding"; decode-event:ipv6.dstopts_only_padding; sid:2200089; rev:1;) # Type 0 Routing header deprecated per RFC 5095 alert ipv6 any any -> any any (msg:"SURICATA RH Type 0"; decode-event:ipv6.rh_type_0; sid:2200093; rev:1;) +# padN option with zero length field +alert ipv6 any any -> any any (msg:"SURICATA zero length padN option"; decode-event:ipv6.zero_len_padn; sid:2200094; rev:1;) alert ipv6 any any -> any any (msg:"SURICATA IPv6 with ICMPv4 header"; decode-event:ipv6.icmpv4; sid:2200090; rev:1;) alert pkthdr any any -> any any (msg:"SURICATA ICMPv4 packet too small"; decode-event:icmpv4.pkt_too_small; sid:2200023; rev:1;) alert pkthdr any any -> any any (msg:"SURICATA ICMPv4 unknown type"; decode-event:icmpv4.unknown_type; sid:2200024; rev:1;) @@ -108,5 +110,5 @@ alert pkthdr any any -> any any (msg:"SURICATA IPv4-in-IPv6 invalid protocol"; d alert pkthdr any any -> any any (msg:"SURICATA IPv6-in-IPv6 packet too short"; decode-event:ipv6.ipv6_in_ipv6_too_small; sid:2200084; rev:1;) alert pkthdr any any -> any any (msg:"SURICATA IPv6-in-IPv6 invalid protocol"; decode-event:ipv6.ipv6_in_ipv6_wrong_version; sid:2200085; rev:1;) -# next sid is 2200094 +# next sid is 2200095 diff --git a/src/decode-events.h b/src/decode-events.h index bedb821db6..998da633e9 100644 --- a/src/decode-events.h +++ b/src/decode-events.h @@ -80,6 +80,7 @@ enum { IPV6_DSTOPTS_ONLY_PADDING, /**< all options in DST opts are padding */ IPV6_EXTHDR_RH_TYPE_0, /**< RH 0 is deprecated as per rfc5095 */ + IPV6_EXTHDR_ZERO_LEN_PADN, /**< padN w/o data (0 len) */ IPV6_WITH_ICMPV4, /**< IPv6 packet with ICMPv4 header */ diff --git a/src/decode-ipv6.c b/src/decode-ipv6.c index 84c1b54d4f..95f3b9dd3e 100644 --- a/src/decode-ipv6.c +++ b/src/decode-ipv6.c @@ -298,15 +298,39 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt continue; } + if (offset + 1 >= optslen) { + ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN); + break; + } + + /* length field for each opt */ + uint8_t ip6_optlen = *(ptr + 1); + + /* see if the optlen from the packet fits the total optslen */ + if ((offset + 1 + ip6_optlen) > optslen) { + ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN); + break; + } + if (*ptr == IPV6OPT_PADN) /* PadN */ { //printf("PadN option\n"); padn_cnt++; + + /* a zero padN len would be weird */ + if (ip6_optlen == 0) + ENGINE_SET_EVENT(p, IPV6_EXTHDR_ZERO_LEN_PADN); } else if (*ptr == IPV6OPT_RA) /* RA */ { ra->ip6ra_type = *(ptr); - ra->ip6ra_len = *(ptr + 1); + ra->ip6ra_len = ip6_optlen; + + if (ip6_optlen < sizeof(ra->ip6ra_value)) { + ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN); + break; + } + memcpy(&ra->ip6ra_value, (ptr + 2), sizeof(ra->ip6ra_value)); ra->ip6ra_value = ntohs(ra->ip6ra_value); //printf("RA option: type %" PRIu32 " len %" PRIu32 " value %" PRIu32 "\n", @@ -316,7 +340,13 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt else if (*ptr == IPV6OPT_JUMBO) /* Jumbo */ { jumbo->ip6j_type = *(ptr); - jumbo->ip6j_len = *(ptr+1); + jumbo->ip6j_len = ip6_optlen; + + if (ip6_optlen < sizeof(jumbo->ip6j_payload_len)) { + ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN); + break; + } + memcpy(&jumbo->ip6j_payload_len, (ptr+2), sizeof(jumbo->ip6j_payload_len)); jumbo->ip6j_payload_len = ntohl(jumbo->ip6j_payload_len); //printf("Jumbo option: type %" PRIu32 " len %" PRIu32 " payload len %" PRIu32 "\n", @@ -325,7 +355,13 @@ DecodeIPV6ExtHdrs(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt else if (*ptr == IPV6OPT_HAO) /* HAO */ { hao->ip6hao_type = *(ptr); - hao->ip6hao_len = *(ptr+1); + hao->ip6hao_len = ip6_optlen; + + if (ip6_optlen < sizeof(hao->ip6hao_hoa)) { + ENGINE_SET_EVENT(p, IPV6_EXTHDR_INVALID_OPTLEN); + break; + } + memcpy(&hao->ip6hao_hoa, (ptr+2), sizeof(hao->ip6hao_hoa)); //printf("HAO option: type %" PRIu32 " len %" PRIu32 " ", // hao->ip6hao_type, hao->ip6hao_len); diff --git a/src/detect-engine-event.h b/src/detect-engine-event.h index ca4d1d05ec..f4b3298c62 100644 --- a/src/detect-engine-event.h +++ b/src/detect-engine-event.h @@ -90,6 +90,7 @@ struct DetectEngineEvents_ { { "ipv6.dstopts_unknown_opt", IPV6_DSTOPTS_UNKNOWN_OPT, }, { "ipv6.dstopts_only_padding", IPV6_DSTOPTS_ONLY_PADDING, }, { "ipv6.rh_type_0", IPV6_EXTHDR_RH_TYPE_0, }, + { "ipv6.zero_len_padn", IPV6_EXTHDR_ZERO_LEN_PADN, }, { "ipv6.icmpv4", IPV6_WITH_ICMPV4, }, /* TCP EVENTS */