Skip to content

Commit db8c799

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-13009/IPv6 mobility: Add a bounds check.
This fixes a buffer over-read discovered by Brian 'geeknik' Carpenter. Add a test using the capture file supplied by the reporter(s). While we're at it: Add a comment giving the RFC for IPv6 mobility headers. Clean up some bounds checks to make it clearer what they're checking, by matching the subsequent EXTRACT_ calls or memcpy. For the binding update, if none of the flag bits are set, don't check the individual flag bits.
1 parent 5edf405 commit db8c799

File tree

4 files changed

+22
-17
lines changed

4 files changed

+22
-17
lines changed

Diff for: print-mobility.c

+20-17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929

3030
/* \summary: IPv6 mobility printer */
31+
/* RFC 3775 */
3132

3233
#ifdef HAVE_CONFIG_H
3334
#include "config.h"
@@ -241,7 +242,7 @@ mobility_print(netdissect_options *ndo,
241242
case IP6M_CAREOF_TEST_INIT:
242243
hlen = IP6M_MINLEN;
243244
if (ndo->ndo_vflag) {
244-
ND_TCHECK2(*mh, hlen + 8);
245+
ND_TCHECK_32BITS(&bp[hlen + 4]);
245246
ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
246247
type == IP6M_HOME_TEST_INIT ? "Home" : "Care-of",
247248
EXTRACT_32BITS(&bp[hlen]),
@@ -255,15 +256,15 @@ mobility_print(netdissect_options *ndo,
255256
ND_PRINT((ndo, " nonce id=0x%x", EXTRACT_16BITS(&mh->ip6m_data16[0])));
256257
hlen = IP6M_MINLEN;
257258
if (ndo->ndo_vflag) {
258-
ND_TCHECK2(*mh, hlen + 8);
259+
ND_TCHECK_32BITS(&bp[hlen + 4]);
259260
ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
260261
type == IP6M_HOME_TEST ? "Home" : "Care-of",
261262
EXTRACT_32BITS(&bp[hlen]),
262263
EXTRACT_32BITS(&bp[hlen + 4])));
263264
}
264265
hlen += 8;
265266
if (ndo->ndo_vflag) {
266-
ND_TCHECK2(*mh, hlen + 8);
267+
ND_TCHECK_32BITS(&bp[hlen + 4]);
267268
ND_PRINT((ndo, " %s Keygen Token=%08x:%08x",
268269
type == IP6M_HOME_TEST ? "Home" : "Care-of",
269270
EXTRACT_32BITS(&bp[hlen]),
@@ -275,37 +276,39 @@ mobility_print(netdissect_options *ndo,
275276
ND_TCHECK(mh->ip6m_data16[0]);
276277
ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&mh->ip6m_data16[0])));
277278
hlen = IP6M_MINLEN;
278-
ND_TCHECK2(*mh, hlen + 1);
279-
if (bp[hlen] & 0xf0)
279+
ND_TCHECK_16BITS(&bp[hlen]);
280+
if (bp[hlen] & 0xf0) {
280281
ND_PRINT((ndo, " "));
281-
if (bp[hlen] & 0x80)
282-
ND_PRINT((ndo, "A"));
283-
if (bp[hlen] & 0x40)
284-
ND_PRINT((ndo, "H"));
285-
if (bp[hlen] & 0x20)
286-
ND_PRINT((ndo, "L"));
287-
if (bp[hlen] & 0x10)
288-
ND_PRINT((ndo, "K"));
282+
if (bp[hlen] & 0x80)
283+
ND_PRINT((ndo, "A"));
284+
if (bp[hlen] & 0x40)
285+
ND_PRINT((ndo, "H"));
286+
if (bp[hlen] & 0x20)
287+
ND_PRINT((ndo, "L"));
288+
if (bp[hlen] & 0x10)
289+
ND_PRINT((ndo, "K"));
290+
}
289291
/* Reserved (4bits) */
290292
hlen += 1;
291293
/* Reserved (8bits) */
292294
hlen += 1;
293-
ND_TCHECK2(*mh, hlen + 2);
295+
ND_TCHECK_16BITS(&bp[hlen]);
294296
/* units of 4 secs */
295297
ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
296298
hlen += 2;
297299
break;
298300
case IP6M_BINDING_ACK:
299301
ND_TCHECK(mh->ip6m_data8[0]);
300302
ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));
303+
ND_TCHECK(mh->ip6m_data8[1]);
301304
if (mh->ip6m_data8[1] & 0x80)
302305
ND_PRINT((ndo, " K"));
303306
/* Reserved (7bits) */
304307
hlen = IP6M_MINLEN;
305-
ND_TCHECK2(*mh, hlen + 2);
308+
ND_TCHECK_16BITS(&bp[hlen]);
306309
ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&bp[hlen])));
307310
hlen += 2;
308-
ND_TCHECK2(*mh, hlen + 2);
311+
ND_TCHECK_16BITS(&bp[hlen]);
309312
/* units of 4 secs */
310313
ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
311314
hlen += 2;
@@ -315,7 +318,7 @@ mobility_print(netdissect_options *ndo,
315318
ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));
316319
/* Reserved */
317320
hlen = IP6M_MINLEN;
318-
ND_TCHECK2(*mh, hlen + 16);
321+
ND_TCHECK2(bp[hlen], 16);
319322
ND_PRINT((ndo, " homeaddr %s", ip6addr_string(ndo, &bp[hlen])));
320323
hlen += 16;
321324
break;

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ ieee802.11_tim_ie_oobr ieee802.11_tim_ie_oobr.pcap ieee802.11_tim_ie_oobr.out
439439
decnet-shorthdr-oobr decnet-shorthdr-oobr.pcap decnet-shorthdr-oobr.out
440440
isakmp-3948-oobr-2 isakmp-3948-oobr-2.pcap isakmp-3948-oobr-2.out
441441
ieee802.11_rates_oobr ieee802.11_rates_oobr.pcap ieee802.11_rates_oobr.out
442+
ipv6-mobility-header-oobr ipv6-mobility-header-oobr.pcap ipv6-mobility-header-oobr.out
442443

443444
# bad packets from Kamil Frankowicz
444445
snmp-heapoverflow-1 snmp-heapoverflow-1.pcap snmp-heapoverflow-1.out

Diff for: tests/ipv6-mobility-header-oobr.out

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
IP6 3030:3030:3030:3030:3030:3030:3030:3030 > 3030:3030:3030:3030:3030:3030:3030:3030: mobility: BA status=48[|MOBILITY]

Diff for: tests/ipv6-mobility-header-oobr.pcap

88 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)