From: Guy Harris Date: Mon, 12 Jun 2017 04:06:55 +0000 (-0700) Subject: CVE-2017-13039/IKEv1: Do more bounds checking. X-Git-Tag: tcpdump-4.99-bp~1880 X-Git-Url: https://git.tcpdump.org/tcpdump/commitdiff_plain/e0a5a02b0fc1900a69d6c37ed0aab36fb8494e6d CVE-2017-13039/IKEv1: Do more bounds checking. Have ikev1_attrmap_print() and ikev1_attr_print() do full bounds checking, and return null on a bounds overflow. Have their callers check for a null return. This fixes a buffer over-read discovered by Bhargava Shastry, SecT/TU Berlin. Add a test using the capture file supplied by the reporter(s), modified so the capture file won't be rejected as an invalid capture. --- diff --git a/print-isakmp.c b/print-isakmp.c index 9de9b75d..013de1c2 100644 --- a/print-isakmp.c +++ b/print-isakmp.c @@ -912,21 +912,25 @@ struct attrmap { static const u_char * ikev1_attrmap_print(netdissect_options *ndo, - const u_char *p, const u_char *ep, + const u_char *p, const u_char *ep2, const struct attrmap *map, size_t nmap) { int totlen; uint32_t t, v; + ND_TCHECK(p[0]); if (p[0] & 0x80) totlen = 4; - else + else { + ND_TCHECK_16BITS(&p[2]); totlen = 4 + EXTRACT_16BITS(&p[2]); - if (ep < p + totlen) { + } + if (ep2 < p + totlen) { ND_PRINT((ndo,"[|attr]")); - return ep + 1; + return ep2 + 1; } + ND_TCHECK_16BITS(&p[0]); ND_PRINT((ndo,"(")); t = EXTRACT_16BITS(&p[0]) & 0x7fff; if (map && t < nmap && map[t].type) @@ -935,47 +939,71 @@ ikev1_attrmap_print(netdissect_options *ndo, ND_PRINT((ndo,"type=#%d ", t)); if (p[0] & 0x80) { ND_PRINT((ndo,"value=")); + ND_TCHECK_16BITS(&p[2]); v = EXTRACT_16BITS(&p[2]); if (map && t < nmap && v < map[t].nvalue && map[t].value[v]) ND_PRINT((ndo,"%s", map[t].value[v])); - else - rawprint(ndo, (const uint8_t *)&p[2], 2); + else { + if (!rawprint(ndo, (const uint8_t *)&p[2], 2)) { + ND_PRINT((ndo,")")); + goto trunc; + } + } } else { - ND_PRINT((ndo,"len=%d value=", EXTRACT_16BITS(&p[2]))); - rawprint(ndo, (const uint8_t *)&p[4], EXTRACT_16BITS(&p[2])); + ND_PRINT((ndo,"len=%d value=", totlen - 4)); + if (!rawprint(ndo, (const uint8_t *)&p[4], totlen - 4)) { + ND_PRINT((ndo,")")); + goto trunc; + } } ND_PRINT((ndo,")")); return p + totlen; + +trunc: + return NULL; } static const u_char * -ikev1_attr_print(netdissect_options *ndo, const u_char *p, const u_char *ep) +ikev1_attr_print(netdissect_options *ndo, const u_char *p, const u_char *ep2) { int totlen; uint32_t t; + ND_TCHECK(p[0]); if (p[0] & 0x80) totlen = 4; - else + else { + ND_TCHECK_16BITS(&p[2]); totlen = 4 + EXTRACT_16BITS(&p[2]); - if (ep < p + totlen) { + } + if (ep2 < p + totlen) { ND_PRINT((ndo,"[|attr]")); - return ep + 1; + return ep2 + 1; } + ND_TCHECK_16BITS(&p[0]); ND_PRINT((ndo,"(")); t = EXTRACT_16BITS(&p[0]) & 0x7fff; ND_PRINT((ndo,"type=#%d ", t)); if (p[0] & 0x80) { ND_PRINT((ndo,"value=")); t = p[2]; - rawprint(ndo, (const uint8_t *)&p[2], 2); + if (!rawprint(ndo, (const uint8_t *)&p[2], 2)) { + ND_PRINT((ndo,")")); + goto trunc; + } } else { - ND_PRINT((ndo,"len=%d value=", EXTRACT_16BITS(&p[2]))); - rawprint(ndo, (const uint8_t *)&p[4], EXTRACT_16BITS(&p[2])); + ND_PRINT((ndo,"len=%d value=", totlen - 4)); + if (!rawprint(ndo, (const uint8_t *)&p[4], totlen - 4)) { + ND_PRINT((ndo,")")); + goto trunc; + } } ND_PRINT((ndo,")")); return p + totlen; + +trunc: + return NULL; } static const u_char * @@ -1256,11 +1284,12 @@ ikev1_t_print(netdissect_options *ndo, u_char tpay _U_, cp = (const u_char *)(p + 1); ep2 = (const u_char *)p + item_len; while (cp < ep && cp < ep2) { - if (map && nmap) { - cp = ikev1_attrmap_print(ndo, cp, (ep < ep2) ? ep : ep2, - map, nmap); - } else - cp = ikev1_attr_print(ndo, cp, (ep < ep2) ? ep : ep2); + if (map && nmap) + cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap); + else + cp = ikev1_attr_print(ndo, cp, ep2); + if (cp == NULL) + goto trunc; } if (ep < ep2) ND_PRINT((ndo,"...")); @@ -1724,8 +1753,11 @@ ikev1_n_print(netdissect_options *ndo, u_char tpay _U_, size_t nmap = sizeof(oakley_t_map)/sizeof(oakley_t_map[0]); ND_PRINT((ndo," attrs=(")); while (cp < ep && cp < ep2) { - cp = ikev1_attrmap_print(ndo, cp, - (ep < ep2) ? ep : ep2, map, nmap); + cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap); + if (cp == NULL) { + ND_PRINT((ndo,")")); + goto trunc; + } } ND_PRINT((ndo,")")); break; @@ -1926,10 +1958,11 @@ ikev2_t_print(netdissect_options *ndo, int tcount, ep2 = (const u_char *)p + item_len; while (cp < ep && cp < ep2) { if (map && nmap) { - cp = ikev1_attrmap_print(ndo, cp, (ep < ep2) ? ep : ep2, - map, nmap); + cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap); } else - cp = ikev1_attr_print(ndo, cp, (ep < ep2) ? ep : ep2); + cp = ikev1_attr_print(ndo, cp, ep2); + if (cp == NULL) + goto trunc; } if (ep < ep2) ND_PRINT((ndo,"...")); diff --git a/tests/TESTLIST b/tests/TESTLIST index bdc7ff40..a830b0cd 100644 --- a/tests/TESTLIST +++ b/tests/TESTLIST @@ -553,6 +553,7 @@ ip6_frag_asan ip6_frag_asan.pcap ip6_frag_asan.out -v radius_attr_asan radius_attr_asan.pcap radius_attr_asan.out -v ospf6_decode_v3_asan ospf6_decode_v3_asan.pcap ospf6_decode_v3_asan.out -v ip_ts_opts_asan ip_ts_opts_asan.pcap ip_ts_opts_asan.out -v +isakmpv1-attr-oobr isakmpv1-attr-oobr.pcap isakmpv1-attr-oobr.out -v # bad packets from Katie Holly mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out diff --git a/tests/isakmpv1-attr-oobr.out b/tests/isakmpv1-attr-oobr.out new file mode 100644 index 00000000..0816735b --- /dev/null +++ b/tests/isakmpv1-attr-oobr.out @@ -0,0 +1,3 @@ +IP (tos 0x60, ttl 254, id 40192, offset 0, flags [+, DF, rsvd], proto UDP (17), length 63264, options (unknown 255 [bad length 18]), bad cksum 8e30 (->f45)!) + 251.73.77.150.32514 > 126.172.128.5.500: isakmp 1.0 msgid 2200af01: phase 2/others ? #40: + (t: #243 id=241 (type=#9472 len=2 value=0619) [|t]) (len mismatch: isakmp 4293885728/ip 2140) diff --git a/tests/isakmpv1-attr-oobr.pcap b/tests/isakmpv1-attr-oobr.pcap new file mode 100644 index 00000000..53619af4 Binary files /dev/null and b/tests/isakmpv1-attr-oobr.pcap differ