CVE-2017-13029/PPP: Fix a bounds check, and clean up other bounds checks.

For configuration protocol options, use ND_TCHECK() and
ND_TCHECK_nBITS() macros, passing them the appropriate pointer argument.
This fixes one case where the ND_TCHECK2() call they replace was not
checking enough bytes.

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-ppp.c b/print-ppp.c
index 134389209fcac0ac885aa8ee15a0670be2d4fd86..d07763cb1cadff5f54cffb561dd7b56333bde99f 100644 (file)
--- a/print-ppp.c
+++ b/print-ppp.c
@@ -611,7 +611,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 6)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 3);
+               ND_TCHECK_24BITS(p + 2);
                ND_PRINT((ndo, ": Vendor: %s (%u)",
                        tok2str(oui_values,"Unknown",EXTRACT_24BITS(p+2)),
                        EXTRACT_24BITS(p + 2)));
@@ -630,7 +630,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 4)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p + 2);
                ND_PRINT((ndo, ": %u", EXTRACT_16BITS(p + 2)));
                break;
        case LCPOPT_ACCM:
@@ -638,7 +638,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 6)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 4);
+               ND_TCHECK_32BITS(p + 2);
                ND_PRINT((ndo, ": 0x%08x", EXTRACT_32BITS(p + 2)));
                break;
        case LCPOPT_AP:
@@ -646,7 +646,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 4)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p + 2);
                ND_PRINT((ndo, ": %s", tok2str(ppptype2str, "Unknown Auth Proto (0x04x)", EXTRACT_16BITS(p + 2))));
 
                switch (EXTRACT_16BITS(p+2)) {
@@ -668,7 +668,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 4)"));
                        return 0;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p+2);
                if (EXTRACT_16BITS(p+2) == PPP_LQM)
                        ND_PRINT((ndo, ": LQR"));
                else
@@ -679,7 +679,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 6)"));
                        return 0;
                }
-               ND_TCHECK2(*(p + 2), 4);
+               ND_TCHECK_32BITS(p + 2);
                ND_PRINT((ndo, ": 0x%08x", EXTRACT_32BITS(p + 2)));
                break;
        case LCPOPT_PFC:
@@ -691,7 +691,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 4)"));
                        return 0;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p + 2);
                ND_PRINT((ndo, ": 0x%04x", EXTRACT_16BITS(p + 2)));
                break;
        case LCPOPT_CBACK:
@@ -710,7 +710,7 @@ print_lcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 4)"));
                        return 0;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p + 2);
                ND_PRINT((ndo, ": %u", EXTRACT_16BITS(p + 2)));
                break;
        case LCPOPT_MLED:
@@ -1055,7 +1055,7 @@ print_ipcp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 4)"));
                        return 0;
                }
-               ND_TCHECK2(*(p + 2), 2);
+               ND_TCHECK_16BITS(p+2);
                compproto = EXTRACT_16BITS(p+2);
 
                ND_PRINT((ndo, ": %s (0x%02x):",
@@ -1241,7 +1241,7 @@ print_ccp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 3)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 1);
+               ND_TCHECK(p[2]);
                ND_PRINT((ndo, ": Version: %u, Dictionary Bits: %u",
                        p[2] >> 5, p[2] & 0x1f));
                break;
@@ -1250,7 +1250,7 @@ print_ccp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 4)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 1);
+               ND_TCHECK(p[3]);
                ND_PRINT((ndo, ": Features: %u, PxP: %s, History: %u, #CTX-ID: %u",
                                (p[2] & 0xc0) >> 6,
                                (p[2] & 0x20) ? "Enabled" : "Disabled",
@@ -1261,7 +1261,7 @@ print_ccp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be >= 4)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 1);
+               ND_TCHECK(p[3]);
                ND_PRINT((ndo, ": Window: %uK, Method: %s (0x%x), MBZ: %u, CHK: %u",
                        (p[2] & 0xf0) >> 4,
                        ((p[2] & 0x0f) == 8) ? "zlib" : "unknown",
@@ -1336,7 +1336,7 @@ print_bacp_config_options(netdissect_options *ndo,
                        ND_PRINT((ndo, " (length bogus, should be = 6)"));
                        return len;
                }
-               ND_TCHECK2(*(p + 2), 4);
+               ND_TCHECK_32BITS(p + 2);
                ND_PRINT((ndo, ": Magic-Num 0x%08x", EXTRACT_32BITS(p + 2)));
                break;
        default:

diff --git a/tests/TESTLIST b/tests/TESTLIST
index 8b90e1f5d543da44c81a9193b5adda6e05cb6175..e553c2caddc89a779d6687c1cd5aaf53775b0370 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -534,6 +534,7 @@ isis_stlv_asan-3    isis_stlv_asan-3.pcap           isis_stlv_asan-3.out    -v
 isis_stlv_asan-4       isis_stlv_asan-4.pcap           isis_stlv_asan-4.out    -v
 lldp_mgmt_addr_tlv_asan        lldp_mgmt_addr_tlv_asan.pcap    lldp_mgmt_addr_tlv_asan.out     -v
 bootp_asan             bootp_asan.pcap                 bootp_asan.out          -v
+ppp_ccp_config_deflate_option_asan     ppp_ccp_config_deflate_option_asan.pcap ppp_ccp_config_deflate_option_asan.out  -v
 
 # RTP tests
 # fuzzed pcap

diff --git a/tests/ppp_ccp_config_deflate_option_asan.out b/tests/ppp_ccp_config_deflate_option_asan.out
new file mode 100644 (file)
index 0000000..423ef05
--- /dev/null
+++ b/tests/ppp_ccp_config_deflate_option_asan.out
@@ -0,0 +1,3 @@
+: CCP, Conf-Request (0x01), id 223, length 125685
+       encoded length 15 (=Option(s) length 11)
+         MVRCA Option (0x18), length 5[|ccp]

diff --git a/tests/ppp_ccp_config_deflate_option_asan.pcap b/tests/ppp_ccp_config_deflate_option_asan.pcap
new file mode 100644 (file)
index 0000000..e3be3ee
Binary files /dev/null and b/tests/ppp_ccp_config_deflate_option_asan.pcap differ