CVE-2017-13690: Out-of-bounds Read (CWE-125)
value: The IKEv2 parser in tcpdump before 4.9.2 has a buffer over-read in print-isakmp.c, several functions.
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-isakmp.c, tests/TESTLIST, tests/isakmp-various-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13690/IKEv2: Fix some bounds checks.

Use a pointer of the correct type in ND_TCHECK(), or use ND_TCHECK2()
and provide the correct length.

While we're at it, remove the blank line between some checks and the
UNALIGNED_MEMCPY()s they protect.

Also, note the places where we print the entire payload.

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).

Diff: diff --git a/print-isakmp.c b/print-isakmp.c
index fbc6c54203..04374b0b32 100644
--- a/print-isakmp.c
+++ b/print-isakmp.c
@@ -1311,10 +1311,11 @@ ikev1_ke_print(netdissect_options *ndo, u_char tpay _U_,
 
 	ND_TCHECK(*ext);
 	UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 	ND_PRINT((ndo," key len=%d", ntohs(e.len) - 4));
 	if (2 < ndo->ndo_vflag && 4 < ntohs(e.len)) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -1513,10 +1514,11 @@ ikev1_cert_print(netdissect_options *ndo, u_char tpay _U_,
 	ND_TCHECK(*p);
 	UNALIGNED_MEMCPY(&cert, ext, sizeof(cert));
 	ND_PRINT((ndo," len=%d", item_len - 4));
 	ND_PRINT((ndo," type=%s", STR_OR_ID((cert.encode), certstr)));
 	if (2 < ndo->ndo_vflag && 4 < item_len) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), item_len - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + item_len;
@@ -1545,10 +1547,11 @@ ikev1_cr_print(netdissect_options *ndo, u_char tpay _U_,
 	ND_TCHECK(*p);
 	UNALIGNED_MEMCPY(&cert, ext, sizeof(cert));
 	ND_PRINT((ndo," len=%d", item_len - 4));
 	ND_PRINT((ndo," type=%s", STR_OR_ID((cert.encode), certstr)));
 	if (2 < ndo->ndo_vflag && 4 < item_len) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), item_len - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + item_len;
@@ -1569,10 +1572,11 @@ ikev1_hash_print(netdissect_options *ndo, u_char tpay _U_,
 
 	ND_TCHECK(*ext);
 	UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 	ND_PRINT((ndo," len=%d", ntohs(e.len) - 4));
 	if (2 < ndo->ndo_vflag && 4 < ntohs(e.len)) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -1593,10 +1597,11 @@ ikev1_sig_print(netdissect_options *ndo, u_char tpay _U_,
 
 	ND_TCHECK(*ext);
 	UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 	ND_PRINT((ndo," len=%d", ntohs(e.len) - 4));
 	if (2 < ndo->ndo_vflag && 4 < ntohs(e.len)) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -1848,10 +1853,11 @@ ikev1_vid_print(netdissect_options *ndo, u_char tpay _U_,
 
 	ND_TCHECK(*ext);
 	UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 	ND_PRINT((ndo," len=%d", ntohs(e.len) - 4));
 	if (2 < ndo->ndo_vflag && 4 < ntohs(e.len)) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -1882,10 +1888,11 @@ ikev2_gen_print(netdissect_options *ndo, u_char tpay,
 	UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 	ikev2_pay_print(ndo, NPSTR(tpay), e.critical);
 
 	ND_PRINT((ndo," len=%d", ntohs(e.len) - 4));
 	if (2 < ndo->ndo_vflag && 4 < ntohs(e.len)) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -2020,11 +2027,10 @@ ikev2_p_print(netdissect_options *ndo, u_char tpay _U_, int pcount _U_,
 		tcount++;
 		ext = (const struct isakmp_gen *)cp;
 		if (prop_length < sizeof(*ext))
 			goto toolong;
 		ND_TCHECK(*ext);
-
 		UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 
 		/*
 		 * Since we can't have a payload length of less than 4 bytes,
 		 * we need to bail out here if the generic header is nonsensical
@@ -2107,11 +2113,10 @@ ikev2_sa_print(netdissect_options *ndo, u_char tpay,
 		pcount++;
 		ext = (const struct isakmp_gen *)cp;
 		if (sa_length < sizeof(*ext))
 			goto toolong;
 		ND_TCHECK(*ext);
-
 		UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 
 		/*
 		 * Since we can't have a payload length of less than 4 bytes,
 		 * we need to bail out here if the generic header is nonsensical
@@ -2168,11 +2173,11 @@ ikev2_ke_print(netdissect_options *ndo, u_char tpay,
 {
 	struct ikev2_ke ke;
 	const struct ikev2_ke *k;
 
 	k = (const struct ikev2_ke *)ext;
-	ND_TCHECK(*ext);
+	ND_TCHECK(*k);
 	UNALIGNED_MEMCPY(&ke, ext, sizeof(ke));
 	ikev2_pay_print(ndo, NPSTR(tpay), ke.h.critical);
 
 	ND_PRINT((ndo," len=%u group=%s", ntohs(ke.h.len) - 8,
 		  STR_OR_ID(ntohs(ke.ke_group), dh_p_map)));
@@ -2193,23 +2198,26 @@ ikev2_ID_print(netdissect_options *ndo, u_char tpay,
 		const struct isakmp_gen *ext,
 		u_int item_len _U_, const u_char *ep _U_,
 		uint32_t phase _U_, uint32_t doi _U_,
 		uint32_t proto _U_, int depth _U_)
 {
+	const struct ikev2_id *idp;
 	struct ikev2_id id;
 	int id_len, idtype_len, i;
 	unsigned int dumpascii, dumphex;
 	const unsigned char *typedata;
 
-	ND_TCHECK(*ext);
+	idp = (const struct ikev2_id *)ext;
+	ND_TCHECK(*idp);
 	UNALIGNED_MEMCPY(&id, ext, sizeof(id));
 	ikev2_pay_print(ndo, NPSTR(tpay), id.h.critical);
 
 	id_len = ntohs(id.h.len);
 
 	ND_PRINT((ndo," len=%d", id_len - 4));
 	if (2 < ndo->ndo_vflag && 4 < id_len) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), id_len - 4))
 			goto trunc;
 	}
 
@@ -2301,11 +2309,11 @@ ikev2_auth_print(netdissect_options *ndo, u_char tpay,
 	const char *v2_auth[]={ "invalid", "rsasig",
 				"shared-secret", "dsssig" };
 	const u_char *authdata = (const u_char*)ext + sizeof(a);
 	unsigned int len;
 
-	ND_TCHECK(*ext);
+	ND_TCHECK2(*ext, sizeof(a));
 	UNALIGNED_MEMCPY(&a, ext, sizeof(a));
 	ikev2_pay_print(ndo, NPSTR(tpay), a.h.critical);
 	len = ntohs(a.h.len);
 
 	/*
@@ -2597,10 +2605,11 @@ ikev2_vid_print(netdissect_options *ndo, u_char tpay,
 	for(i=0; i<len; i++) {
 		if(ND_ISPRINT(vid[i])) ND_PRINT((ndo, "%c", vid[i]));
 		else ND_PRINT((ndo, "."));
 	}
 	if (2 < ndo->ndo_vflag && 4 < len) {
+		/* Print the entire payload in hex */
 		ND_PRINT((ndo," "));
 		if (!rawprint(ndo, (const uint8_t *)(ext + 1), ntohs(e.len) - 4))
 			goto trunc;
 	}
 	return (const u_char *)ext + ntohs(e.len);
@@ -2762,11 +2771,10 @@ ikev1_sub_print(netdissect_options *ndo,
 
 	cp = (const u_char *)ext;
 
 	while (np) {
 		ND_TCHECK(*ext);
-
 		UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 
 		ND_TCHECK2(*ext, ntohs(e.len));
 
 		depth++;
@@ -2930,11 +2938,10 @@ ikev2_sub_print(netdissect_options *ndo,
 	struct isakmp_gen e;
 
 	cp = (const u_char *)ext;
 	while (np) {
 		ND_TCHECK(*ext);
-
 		UNALIGNED_MEMCPY(&e, ext, sizeof(e));
 
 		ND_TCHECK2(*ext, ntohs(e.len));
 
 		depth++;
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 0e70972101..bd6073d71f 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -576,10 +576,11 @@ bgp_vpn_rt-oobr	bgp_vpn_rt-oobr.pcap	bgp_vpn_rt-oobr.out	-v -c1
 cfm_sender_id-oobr	cfm_sender_id-oobr.pcap	cfm_sender_id-oobr.out	-v -c1
 isis-extd-isreach-oobr	isis-extd-isreach-oobr.pcap	isis-extd-isreach-oobr.out -v -c4
 olsr-oobr-1		olsr-oobr-1.pcap		olsr-oobr-1.out	-v
 olsr-oobr-2		olsr-oobr-2.pcap		olsr-oobr-2.out	-v
 ikev1_id_ipv6_addr_subnet-oobr	ikev1_id_ipv6_addr_subnet-oobr.pcap	ikev1_id_ipv6_addr_subnet-oobr.out	-v
+isakmp-various-oobr	isakmp-various-oobr.pcap	isakmp-various-oobr.out	-v
 
 # bad packets from Katie Holly
 mlppp-oobr		mlppp-oobr.pcap			mlppp-oobr.out
 
 # bad packets from Kim Gwan Yeong
diff --git a/tests/isakmp-various-oobr.out b/tests/isakmp-various-oobr.out
new file mode 100644
index 0000000000..cf1b1e046d
--- /dev/null
+++ b/tests/isakmp-various-oobr.out
@@ -0,0 +1,5 @@
+IP (tos 0xfb,CE, ttl 17, id 21263, offset 0, flags [+, DF, rsvd], proto UDP (17), length 296, bad cksum 1ff (->f67d)!)
+    16.0.128.20.500 > 12.251.225.45.49152: isakmp 1.0 msgid 10101010: phase 2/others ? #16[]:
+    ( [|v2ke]) (len mismatch: isakmp 2130706432/ip 268)
+IP (tos 0x12,ECT(0), ttl 17, id 21263, offset 4096, flags [DF, rsvd], proto UDP (17), length 296, bad cksum 1ff (->939f)!)
+    0.0.0.5 > 0.0.0.0: ip-proto-17
diff --git a/tests/isakmp-various-oobr.pcap b/tests/isakmp-various-oobr.pcap
new file mode 100644
index 0000000000..b81d3f5d37
Binary files /dev/null and b/tests/isakmp-various-oobr.pcap differ

Vulnerable code: 

	ND_TCHECK(*ext);
	ND_TCHECK(*ext);
	ND_TCHECK(*ext);


Fixed code: 		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
		/* Print the entire payload in hex */
	ND_TCHECK(*k);
	const struct ikev2_id *idp;
	idp = (const struct ikev2_id *)ext;
	ND_TCHECK(*idp);
		/* Print the entire payload in hex */
	ND_TCHECK2(*ext, sizeof(a));
		/* Print the entire payload in hex */
isakmp-various-oobr	isakmp-various-oobr.pcap	isakmp-various-oobr.out	-v
IP (tos 0xfb,CE, ttl 17, id 21263, offset 0, flags [+, DF, rsvd], proto UDP (17), length 296, bad cksum 1ff (->f67d)!)
    16.0.128.20.500 > 12.251.225.45.49152: isakmp 1.0 msgid 10101010: phase 2/others ? #16[]:
    ( [|v2ke]) (len mismatch: isakmp 2130706432/ip 268)
IP (tos 0x12,ECT(0), ttl 17, id 21263, offset 4096, flags [DF, rsvd], proto UDP (17), length 296, bad cksum 1ff (->939f)!)
    0.0.0.5 > 0.0.0.0: ip-proto-17