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.

diff --git a/print-mobility.c b/print-mobility.c
index ca5541330ca981834352ab6b50cba95ad3d0f5a3..d3ca0cabd7e771ff9679b0fd989c7af74c6a26a6 100644 (file)
--- a/print-mobility.c
+++ b/print-mobility.c
@@ -28,6 +28,7 @@
  */
 
 /* \summary: IPv6 mobility printer */
  */
 
 /* \summary: IPv6 mobility printer */

+/* RFC 3775 */

 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"


@@ -241,7 +242,7 @@ mobility_print(netdissect_options *ndo,
        case IP6M_CAREOF_TEST_INIT:
                hlen = IP6M_MINLEN;
                if (ndo->ndo_vflag) {
        case IP6M_CAREOF_TEST_INIT:
                hlen = IP6M_MINLEN;
                if (ndo->ndo_vflag) {

-                       ND_TCHECK2(*mh, hlen + 8);
+                       ND_TCHECK_32BITS(&bp[hlen + 4]);

                        ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
                               type == IP6M_HOME_TEST_INIT ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),
                        ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
                               type == IP6M_HOME_TEST_INIT ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),


@@ -255,7 +256,7 @@ mobility_print(netdissect_options *ndo,
                ND_PRINT((ndo, " nonce id=0x%x", EXTRACT_16BITS(&mh->ip6m_data16[0])));
                hlen = IP6M_MINLEN;
                if (ndo->ndo_vflag) {
                ND_PRINT((ndo, " nonce id=0x%x", EXTRACT_16BITS(&mh->ip6m_data16[0])));
                hlen = IP6M_MINLEN;
                if (ndo->ndo_vflag) {

-                       ND_TCHECK2(*mh, hlen + 8);
+                       ND_TCHECK_32BITS(&bp[hlen + 4]);

                        ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
                               type == IP6M_HOME_TEST ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),
                        ND_PRINT((ndo, " %s Init Cookie=%08x:%08x",
                               type == IP6M_HOME_TEST ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),


@@ -263,7 +264,7 @@ mobility_print(netdissect_options *ndo,
                }
                hlen += 8;
                if (ndo->ndo_vflag) {
                }
                hlen += 8;
                if (ndo->ndo_vflag) {

-                       ND_TCHECK2(*mh, hlen + 8);
+                       ND_TCHECK_32BITS(&bp[hlen + 4]);

                        ND_PRINT((ndo, " %s Keygen Token=%08x:%08x",
                               type == IP6M_HOME_TEST ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),
                        ND_PRINT((ndo, " %s Keygen Token=%08x:%08x",
                               type == IP6M_HOME_TEST ? "Home" : "Care-of",
                               EXTRACT_32BITS(&bp[hlen]),


@@ -275,22 +276,23 @@ mobility_print(netdissect_options *ndo,
                ND_TCHECK(mh->ip6m_data16[0]);
                ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&mh->ip6m_data16[0])));
                hlen = IP6M_MINLEN;
                ND_TCHECK(mh->ip6m_data16[0]);
                ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&mh->ip6m_data16[0])));
                hlen = IP6M_MINLEN;

-               ND_TCHECK2(*mh, hlen + 1);
-               if (bp[hlen] & 0xf0)
+               ND_TCHECK_16BITS(&bp[hlen]);
+               if (bp[hlen] & 0xf0) {

                        ND_PRINT((ndo, " "));
                        ND_PRINT((ndo, " "));

-               if (bp[hlen] & 0x80)
-                       ND_PRINT((ndo, "A"));
-               if (bp[hlen] & 0x40)
-                       ND_PRINT((ndo, "H"));
-               if (bp[hlen] & 0x20)
-                       ND_PRINT((ndo, "L"));
-               if (bp[hlen] & 0x10)
-                       ND_PRINT((ndo, "K"));
+                       if (bp[hlen] & 0x80)
+                               ND_PRINT((ndo, "A"));
+                       if (bp[hlen] & 0x40)
+                               ND_PRINT((ndo, "H"));
+                       if (bp[hlen] & 0x20)
+                               ND_PRINT((ndo, "L"));
+                       if (bp[hlen] & 0x10)
+                               ND_PRINT((ndo, "K"));
+               }

                /* Reserved (4bits) */
                hlen += 1;
                /* Reserved (8bits) */
                hlen += 1;
                /* Reserved (4bits) */
                hlen += 1;
                /* Reserved (8bits) */
                hlen += 1;

-               ND_TCHECK2(*mh, hlen + 2);
+               ND_TCHECK_16BITS(&bp[hlen]);

                /* units of 4 secs */
                ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
                hlen += 2;
                /* units of 4 secs */
                ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
                hlen += 2;


@@ -298,14 +300,15 @@ mobility_print(netdissect_options *ndo,
        case IP6M_BINDING_ACK:
                ND_TCHECK(mh->ip6m_data8[0]);
                ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));
        case IP6M_BINDING_ACK:
                ND_TCHECK(mh->ip6m_data8[0]);
                ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));

+               ND_TCHECK(mh->ip6m_data8[1]);

                if (mh->ip6m_data8[1] & 0x80)
                        ND_PRINT((ndo, " K"));
                /* Reserved (7bits) */
                hlen = IP6M_MINLEN;
                if (mh->ip6m_data8[1] & 0x80)
                        ND_PRINT((ndo, " K"));
                /* Reserved (7bits) */
                hlen = IP6M_MINLEN;

-               ND_TCHECK2(*mh, hlen + 2);
+               ND_TCHECK_16BITS(&bp[hlen]);

                ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&bp[hlen])));
                hlen += 2;
                ND_PRINT((ndo, " seq#=%u", EXTRACT_16BITS(&bp[hlen])));
                hlen += 2;

-               ND_TCHECK2(*mh, hlen + 2);
+               ND_TCHECK_16BITS(&bp[hlen]);

                /* units of 4 secs */
                ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
                hlen += 2;
                /* units of 4 secs */
                ND_PRINT((ndo, " lifetime=%u", EXTRACT_16BITS(&bp[hlen]) << 2));
                hlen += 2;


@@ -315,7 +318,7 @@ mobility_print(netdissect_options *ndo,
                ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));
                /* Reserved */
                hlen = IP6M_MINLEN;
                ND_PRINT((ndo, " status=%u", mh->ip6m_data8[0]));
                /* Reserved */
                hlen = IP6M_MINLEN;

-               ND_TCHECK2(*mh, hlen + 16);
+               ND_TCHECK2(bp[hlen], 16);

                ND_PRINT((ndo, " homeaddr %s", ip6addr_string(ndo, &bp[hlen])));
                hlen += 16;
                break;
                ND_PRINT((ndo, " homeaddr %s", ip6addr_string(ndo, &bp[hlen])));
                hlen += 16;
                break;

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 17ee6fb730abe7e110fd18c2ce878ccdebfb3721..5dad008ef5b7a779aeb943dfad9618ce1aa1d74e 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -439,6 +439,7 @@ ieee802.11_tim_ie_oobr      ieee802.11_tim_ie_oobr.pcap     ieee802.11_tim_ie_oobr.out
 decnet-shorthdr-oobr   decnet-shorthdr-oobr.pcap       decnet-shorthdr-oobr.out
 isakmp-3948-oobr-2     isakmp-3948-oobr-2.pcap         isakmp-3948-oobr-2.out
 ieee802.11_rates_oobr  ieee802.11_rates_oobr.pcap      ieee802.11_rates_oobr.out
 decnet-shorthdr-oobr   decnet-shorthdr-oobr.pcap       decnet-shorthdr-oobr.out
 isakmp-3948-oobr-2     isakmp-3948-oobr-2.pcap         isakmp-3948-oobr-2.out
 ieee802.11_rates_oobr  ieee802.11_rates_oobr.pcap      ieee802.11_rates_oobr.out

+ipv6-mobility-header-oobr      ipv6-mobility-header-oobr.pcap  ipv6-mobility-header-oobr.out

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

diff --git a/tests/ipv6-mobility-header-oobr.out b/tests/ipv6-mobility-header-oobr.out
new file mode 100644 (file)
index 0000000..8188f8a
--- /dev/null
+++ b/tests/ipv6-mobility-header-oobr.out
@@ -0,0 +1 @@
+IP6 3030:3030:3030:3030:3030:3030:3030:3030 > 3030:3030:3030:3030:3030:3030:3030:3030: mobility: BA status=48[|MOBILITY]

diff --git a/tests/ipv6-mobility-header-oobr.pcap b/tests/ipv6-mobility-header-oobr.pcap
new file mode 100644 (file)
index 0000000..87f6ef1
Binary files /dev/null and b/tests/ipv6-mobility-header-oobr.pcap differ