CVE-2017-13029: Out-of-bounds Read (CWE-125)
value: The PPP parser in tcpdump before 4.9.2 has a buffer over-read in print-ppp.c:print_ccp_config_options().
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2017-09-14T06:29Z
Repository: https://github.com/the-tcpdump-group/tcpdump
Language: Other
Files changed: print-ppp.c, tests/TESTLIST, tests/ppp_ccp_config_deflate_option_asan.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: 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: diff --git a/print-ppp.c b/print-ppp.c
index 134389209f..d07763cb1c 100644
--- a/print-ppp.c
+++ b/print-ppp.c
@@ -609,11 +609,11 @@ print_lcp_config_options(netdissect_options *ndo,
 	case LCPOPT_VEXT:
 		if (len < 6) {
 			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)));
 #if 0
 		ND_TCHECK(p[5]);
@@ -628,27 +628,27 @@ print_lcp_config_options(netdissect_options *ndo,
 	case LCPOPT_MRU:
 		if (len != 4) {
 			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:
 		if (len != 6) {
 			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:
 		if (len < 4) {
 			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)) {
 		case PPP_CHAP:
 			ND_TCHECK(p[4]);
@@ -666,22 +666,22 @@ print_lcp_config_options(netdissect_options *ndo,
 	case LCPOPT_QP:
 		if (len < 4) {
 			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
 			ND_PRINT((ndo, ": unknown"));
 		break;
 	case LCPOPT_MN:
 		if (len != 6) {
 			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:
 		break;
 	case LCPOPT_ACFC:
@@ -689,11 +689,11 @@ print_lcp_config_options(netdissect_options *ndo,
 	case LCPOPT_LD:
 		if (len != 4) {
 			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:
 		if (len < 3) {
 			ND_PRINT((ndo, " (length bogus, should be >= 3)"));
@@ -708,11 +708,11 @@ print_lcp_config_options(netdissect_options *ndo,
 	case LCPOPT_MLMRRU:
 		if (len != 4) {
 			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:
 		if (len < 3) {
 			ND_PRINT((ndo, " (length bogus, should be >= 3)"));
@@ -1053,11 +1053,11 @@ print_ipcp_config_options(netdissect_options *ndo,
 	case IPCPOPT_IPCOMP:
 		if (len < 4) {
 			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):",
 		          tok2str(ipcpopt_compproto_values, "Unknown", compproto),
 		          compproto));
@@ -1239,31 +1239,31 @@ print_ccp_config_options(netdissect_options *ndo,
 	case CCPOPT_BSDCOMP:
 		if (len < 3) {
 			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;
 	case CCPOPT_MVRCA:
 		if (len < 4) {
 			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",
 				p[2] & 0x1f, p[3]));
 		break;
 	case CCPOPT_DEFLATE:
 		if (len < 4) {
 			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",
 			p[2] & 0x0f, (p[3] & 0xfc) >> 2, p[3] & 0x03));
 		break;
@@ -1334,11 +1334,11 @@ print_bacp_config_options(netdissect_options *ndo,
 	case BACPOPT_FPEER:
 		if (len != 6) {
 			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:
 		/*
 		 * Unknown option; dump it as raw bytes now if we're
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 8b90e1f5d5..e553c2cadd 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -532,10 +532,11 @@ isis_stlv_asan		isis_stlv_asan.pcap		isis_stlv_asan.out	-v
 isis_stlv_asan-2	isis_stlv_asan-2.pcap		isis_stlv_asan-2.out	-v
 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
 rtp-seg-fault-1  rtp-seg-fault-1.pcap  rtp-seg-fault-1.out  -v -T rtp
 rtp-seg-fault-2  rtp-seg-fault-2.pcap  rtp-seg-fault-2.out  -v -T rtp
diff --git a/tests/ppp_ccp_config_deflate_option_asan.out b/tests/ppp_ccp_config_deflate_option_asan.out
new file mode 100644
index 0000000000..423ef050d9
--- /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
index 0000000000..e3be3ee458
Binary files /dev/null and b/tests/ppp_ccp_config_deflate_option_asan.pcap differ

Vulnerable code: 		ND_TCHECK2(*(p + 2), 3);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 4);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 4);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 2);
		ND_TCHECK2(*(p + 2), 1);
		ND_TCHECK2(*(p + 2), 1);
		ND_TCHECK2(*(p + 2), 1);
		ND_TCHECK2(*(p + 2), 4);
Fixed code: 		ND_TCHECK_24BITS(p + 2);
		ND_TCHECK_16BITS(p + 2);
		ND_TCHECK_32BITS(p + 2);
		ND_TCHECK_16BITS(p + 2);
		ND_TCHECK_16BITS(p+2);
		ND_TCHECK_32BITS(p + 2);
		ND_TCHECK_16BITS(p + 2);
		ND_TCHECK_16BITS(p + 2);
		ND_TCHECK_16BITS(p+2);
		ND_TCHECK(p[2]);
		ND_TCHECK(p[3]);
		ND_TCHECK(p[3]);
		ND_TCHECK_32BITS(p + 2);
ppp_ccp_config_deflate_option_asan	ppp_ccp_config_deflate_option_asan.pcap	ppp_ccp_config_deflate_option_asan.out	-v
: CCP, Conf-Request (0x01), id 223, length 125685
	encoded length 15 (=Option(s) length 11)
	  MVRCA Option (0x18), length 5[|ccp]