CVE-2017-12992: Out-of-bounds Read (CWE-125)
value: The RIPng parser in tcpdump before 4.9.2 has a buffer over-read in print-ripng.c:ripng_print().
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-ripng.c, tests/TESTLIST, tests/hoobr_ripng_print.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-12992/RIPng: Clean up bounds checking.

Do bounds checking as we access items.

Scan the list of netinfo6 entries based on the supplied packet length,
without taking the captured length into account; let the aforementioned
bounds checking handle that.

This fixes a buffer over-read discovered by Kamil Frankowicz.

Add a test using the capture file supplied by the reporter(s).

Diff: diff --git a/print-ripng.c b/print-ripng.c
index 25e9bbca0..7113239ad 100644
--- a/print-ripng.c
+++ b/print-ripng.c
@@ -108,67 +108,76 @@ rip6_entry_print(netdissect_options *ndo, register const struct netinfo6 *ni, in
 void
 ripng_print(netdissect_options *ndo, const u_char *dat, unsigned int length)
 {
 	register const struct rip6 *rp = (const struct rip6 *)dat;
 	register const struct netinfo6 *ni;
-	register u_int amt;
-	register u_int i;
-	int j;
-	int trunc;
-
-	if (ndo->ndo_snapend < dat)
-		return;
-	amt = ndo->ndo_snapend - dat;
-	i = min(length, amt);
-	if (i < (sizeof(struct rip6) - sizeof(struct netinfo6)))
-		return;
-	i -= (sizeof(struct rip6) - sizeof(struct netinfo6));
+	unsigned int length_left;
+	u_int j;
 
+	ND_TCHECK(rp->rip6_cmd);
 	switch (rp->rip6_cmd) {
 
 	case RIP6_REQUEST:
-		j = length / sizeof(*ni);
-		if (j == 1
-		    &&  rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
-		    &&  IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
-			ND_PRINT((ndo, " ripng-req dump"));
-			break;
+		length_left = length;
+		if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
+			goto trunc;
+		length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
+ 		j = length_left / sizeof(*ni);
+		if (j == 1) {
+			ND_TCHECK(rp->rip6_nets);
+			if (rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
+			    &&  IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
+				ND_PRINT((ndo, " ripng-req dump"));
+				break;
+			}
 		}
-		if (j * sizeof(*ni) != length - 4)
-			ND_PRINT((ndo, " ripng-req %d[%u]:", j, length));
+		if (j * sizeof(*ni) != length_left)
+			ND_PRINT((ndo, " ripng-req %u[%u]:", j, length));
 		else
-			ND_PRINT((ndo, " ripng-req %d:", j));
-		trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
-		for (ni = rp->rip6_nets; i >= sizeof(*ni);
-		    i -= sizeof(*ni), ++ni) {
+			ND_PRINT((ndo, " ripng-req %u:", j));
+		for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
+		    length_left -= sizeof(*ni), ++ni) {
+			ND_TCHECK(*ni);
 			if (ndo->ndo_vflag > 1)
 				ND_PRINT((ndo, "\n\t"));
 			else
 				ND_PRINT((ndo, " "));
 			rip6_entry_print(ndo, ni, 0);
 		}
+		if (length_left != 0)
+			goto trunc;
 		break;
 	case RIP6_RESPONSE:
-		j = length / sizeof(*ni);
-		if (j * sizeof(*ni) != length - 4)
+		length_left = length;
+		if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
+			goto trunc;
+		length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
+		j = length_left / sizeof(*ni);
+		if (j * sizeof(*ni) != length_left)
 			ND_PRINT((ndo, " ripng-resp %d[%u]:", j, length));
 		else
 			ND_PRINT((ndo, " ripng-resp %d:", j));
-		trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
-		for (ni = rp->rip6_nets; i >= sizeof(*ni);
-		    i -= sizeof(*ni), ++ni) {
+		for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
+		    length_left -= sizeof(*ni), ++ni) {
+			ND_TCHECK(*ni);
 			if (ndo->ndo_vflag > 1)
 				ND_PRINT((ndo, "\n\t"));
 			else
 				ND_PRINT((ndo, " "));
 			rip6_entry_print(ndo, ni, ni->rip6_metric);
 		}
-		if (trunc)
-			ND_PRINT((ndo, "[|ripng]"));
+		if (length_left != 0)
+			goto trunc;
 		break;
 	default:
 		ND_PRINT((ndo, " ripng-%d ?? %u", rp->rip6_cmd, length));
 		break;
 	}
+	ND_TCHECK(rp->rip6_vers);
 	if (rp->rip6_vers != RIP6_VERSION)
 		ND_PRINT((ndo, " [vers %d]", rp->rip6_vers));
+	return;
+
+trunc:
+	ND_PRINT((ndo, "[|ripng]"));
+	return;
 }
diff --git a/tests/TESTLIST b/tests/TESTLIST
index ecf54f69b..142dd45a9 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -448,10 +448,11 @@ hoobr_safeputs		hoobr_safeputs.pcap		hoobr_safeputs.out
 isakmp-rfc3948-oobr	isakmp-rfc3948-oobr.pcap	isakmp-rfc3948-oobr.out
 isoclns-oobr		isoclns-oobr.pcap		isoclns-oobr.out
 nfs-attr-oobr		nfs-attr-oobr.pcap		nfs-attr-oobr.out
 decnet-oobr		decnet-oobr.pcap		decnet-oobr.out
 oobr_parse_elements	oobr_parse_elements.pcap	oobr_parse_elements.out
+hoobr_ripng_print	hoobr_ripng_print.pcap		hoobr_ripng_print.out
 
 # bad packets from Wilfried Kirsch
 slip-bad-direction	slip-bad-direction.pcap		slip-bad-direction.out	-ve
 
 # bad packets from Otto Airamo and Antti Levomäki
diff --git a/tests/hoobr_ripng_print.out b/tests/hoobr_ripng_print.out
new file mode 100644
index 000000000..c2d66a066
--- /dev/null
+++ b/tests/hoobr_ripng_print.out
@@ -0,0 +1 @@
+IP 48.48.48.48.521 > 48.48.48.48.12336: [|ripng]
diff --git a/tests/hoobr_ripng_print.pcap b/tests/hoobr_ripng_print.pcap
new file mode 100644
index 000000000..7eabe3673
Binary files /dev/null and b/tests/hoobr_ripng_print.pcap differ

Vulnerable code: 	register u_int amt;
	register u_int i;
	int j;
	int trunc;

	if (ndo->ndo_snapend < dat)
		return;
	amt = ndo->ndo_snapend - dat;
	i = min(length, amt);
	if (i < (sizeof(struct rip6) - sizeof(struct netinfo6)))
		return;
	i -= (sizeof(struct rip6) - sizeof(struct netinfo6));
		j = length / sizeof(*ni);
		if (j == 1
		    &&  rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
		    &&  IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
			ND_PRINT((ndo, " ripng-req dump"));
			break;
		if (j * sizeof(*ni) != length - 4)
			ND_PRINT((ndo, " ripng-req %d[%u]:", j, length));
			ND_PRINT((ndo, " ripng-req %d:", j));
		trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
		for (ni = rp->rip6_nets; i >= sizeof(*ni);
		    i -= sizeof(*ni), ++ni) {
		j = length / sizeof(*ni);
		if (j * sizeof(*ni) != length - 4)
		trunc = ((i / sizeof(*ni)) * sizeof(*ni) != i);
		for (ni = rp->rip6_nets; i >= sizeof(*ni);
		    i -= sizeof(*ni), ++ni) {
		if (trunc)
			ND_PRINT((ndo, "[|ripng]"));
Fixed code: 	unsigned int length_left;
	u_int j;
	ND_TCHECK(rp->rip6_cmd);
		length_left = length;
		if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
			goto trunc;
		length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
 		j = length_left / sizeof(*ni);
		if (j == 1) {
			ND_TCHECK(rp->rip6_nets);
			if (rp->rip6_nets->rip6_metric == HOPCNT_INFINITY6
			    &&  IN6_IS_ADDR_UNSPECIFIED(&rp->rip6_nets->rip6_dest)) {
				ND_PRINT((ndo, " ripng-req dump"));
				break;
			}
		if (j * sizeof(*ni) != length_left)
			ND_PRINT((ndo, " ripng-req %u[%u]:", j, length));
			ND_PRINT((ndo, " ripng-req %u:", j));
		for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
		    length_left -= sizeof(*ni), ++ni) {
			ND_TCHECK(*ni);
		if (length_left != 0)
			goto trunc;
		length_left = length;
		if (length_left < (sizeof(struct rip6) - sizeof(struct netinfo6)))
			goto trunc;
		length_left -= (sizeof(struct rip6) - sizeof(struct netinfo6));
		j = length_left / sizeof(*ni);
		if (j * sizeof(*ni) != length_left)
		for (ni = rp->rip6_nets; length_left >= sizeof(*ni);
		    length_left -= sizeof(*ni), ++ni) {
			ND_TCHECK(*ni);
		if (length_left != 0)
			goto trunc;
	ND_TCHECK(rp->rip6_vers);
	return;

trunc:
	ND_PRINT((ndo, "[|ripng]"));
	return;
hoobr_ripng_print	hoobr_ripng_print.pcap		hoobr_ripng_print.out
IP 48.48.48.48.521 > 48.48.48.48.12336: [|ripng]