CVE-2017-12897: Out-of-bounds Read (CWE-125)
value: The ISO CLNS parser in tcpdump before 4.9.2 has a buffer over-read in print-isoclns.c:isoclns_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: C
Files changed: netdissect.h, print-atm.c, print-chdlc.c, print-ether.c, print-fr.c, print-gre.c, print-isoclns.c, print-juniper.c, print-llc.c, print-mpls.c, print-null.c, print-ppp.c, tests/TESTLIST, tests/isoclns-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-12897/ISO CLNS: Use ND_TTEST() for the bounds checks in isoclns_print().

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

Don't pass the remaining caplen - that's too hard to get right, and we
were getting it wrong in at least one case; just use ND_TTEST().

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

Diff: diff --git a/netdissect.h b/netdissect.h
index 26d6d47b3..9060d1500 100644
--- a/netdissect.h
+++ b/netdissect.h
@@ -510,11 +510,11 @@ extern void ip_print_inner(netdissect_options *, const u_char *, u_int, u_int nh
 extern void ipcomp_print(netdissect_options *, register const u_char *);
 extern void ipx_netbios_print(netdissect_options *, const u_char *, u_int);
 extern void ipx_print(netdissect_options *, const u_char *, u_int);
 extern void isakmp_print(netdissect_options *, const u_char *, u_int, const u_char *);
 extern void isakmp_rfc3948_print(netdissect_options *, const u_char *, u_int, const u_char *);
-extern void isoclns_print(netdissect_options *, const u_char *, u_int, u_int);
+extern void isoclns_print(netdissect_options *, const u_char *, u_int);
 extern void krb_print(netdissect_options *, const u_char *);
 extern void l2tp_print(netdissect_options *, const u_char *, u_int);
 extern void lane_print(netdissect_options *, const u_char *, u_int, u_int);
 extern void ldp_print(netdissect_options *, const u_char *, u_int);
 extern void lisp_print(netdissect_options *, const u_char *, u_int);
diff --git a/print-atm.c b/print-atm.c
index 7c12262c9..12ada49df 100644
--- a/print-atm.c
+++ b/print-atm.c
@@ -260,11 +260,11 @@ atm_if_print(netdissect_options *ndo,
 
         /* Cisco Style NLPID ? */
         if (*p == LLC_UI) {
             if (ndo->ndo_eflag)
                 ND_PRINT((ndo, "CNLPID "));
-            isoclns_print(ndo, p + 1, length - 1, caplen - 1);
+            isoclns_print(ndo, p + 1, length - 1);
             return hdrlen;
         }
 
 	/*
 	 * Must have at least a DSAP, an SSAP, and the first byte of the
diff --git a/print-chdlc.c b/print-chdlc.c
index ca96cc506..24acfbd2e 100644
--- a/print-chdlc.c
+++ b/print-chdlc.c
@@ -95,13 +95,13 @@ chdlc_print(netdissect_options *ndo, register const u_char *p, u_int length)
                     goto trunc;
                 ND_TCHECK_16BITS(p);
                 if (*(p+1) == 0x81 ||
                     *(p+1) == 0x82 ||
                     *(p+1) == 0x83)
-                    isoclns_print(ndo, p + 1, length - 1, ndo->ndo_snapend - p - 1);
+                    isoclns_print(ndo, p + 1, length - 1);
                 else
-                    isoclns_print(ndo, p, length, ndo->ndo_snapend - p);
+                    isoclns_print(ndo, p, length);
                 break;
 	default:
                 if (!ndo->ndo_eflag)
                         ND_PRINT((ndo, "unknown CHDLC protocol (0x%04x)", proto));
                 break;
diff --git a/print-ether.c b/print-ether.c
index 57c07ce93..33a135e0c 100644
--- a/print-ether.c
+++ b/print-ether.c
@@ -365,11 +365,11 @@ ethertype_print(netdissect_options *ndo,
 	case ETHERTYPE_ISO:
 		if (length == 0 || caplen == 0) {
 			ND_PRINT((ndo, " [|osi]"));
 			return (1);
 		}
-		isoclns_print(ndo, p + 1, length - 1, caplen - 1);
+		isoclns_print(ndo, p + 1, length - 1);
 		return(1);
 
 	case ETHERTYPE_PPPOED:
 	case ETHERTYPE_PPPOES:
 	case ETHERTYPE_PPPOED2:
diff --git a/print-fr.c b/print-fr.c
index dad8173c7..f1a8b21e5 100644
--- a/print-fr.c
+++ b/print-fr.c
@@ -327,11 +327,11 @@ fr_print(netdissect_options *ndo,
 		break;
 
 	case NLPID_CLNP:
 	case NLPID_ESIS:
 	case NLPID_ISIS:
-		isoclns_print(ndo, p - 1, length + 1, ndo->ndo_snapend - p + 1); /* OSI printers need the NLPID field */
+		isoclns_print(ndo, p - 1, length + 1); /* OSI printers need the NLPID field */
 		break;
 
 	case NLPID_SNAP:
 		if (snap_print(ndo, p, length, ndo->ndo_snapend - p, NULL, NULL, 0) == 0) {
 			/* ether_type not known, print raw packet */
diff --git a/print-gre.c b/print-gre.c
index 4664bd3f1..47a000cf5 100644
--- a/print-gre.c
+++ b/print-gre.c
@@ -225,11 +225,11 @@ gre_print_0(netdissect_options *ndo, const u_char *bp, u_int length)
 		break;
 	case ETHERTYPE_ATALK:
 		atalk_print(ndo, bp, len);
 		break;
 	case ETHERTYPE_GRE_ISO:
-		isoclns_print(ndo, bp, len, ndo->ndo_snapend - bp);
+		isoclns_print(ndo, bp, len);
 		break;
 	case ETHERTYPE_TEB:
 		ether_print(ndo, bp, len, ndo->ndo_snapend - bp, NULL, NULL);
 		break;
 	default:
diff --git a/print-isoclns.c b/print-isoclns.c
index a3583e77b..95339a0ad 100644
--- a/print-isoclns.c
+++ b/print-isoclns.c
@@ -668,14 +668,13 @@ struct isis_tlv_lsp {
 #define ISIS_LSP_HEADER_SIZE (sizeof(struct isis_lsp_header))
 #define ISIS_CSNP_HEADER_SIZE (sizeof(struct isis_csnp_header))
 #define ISIS_PSNP_HEADER_SIZE (sizeof(struct isis_psnp_header))
 
 void
-isoclns_print(netdissect_options *ndo,
-              const uint8_t *p, u_int length, u_int caplen)
+isoclns_print(netdissect_options *ndo, const uint8_t *p, u_int length)
 {
-	if (caplen <= 1) { /* enough bytes on the wire ? */
+	if (!ND_TTEST(*p)) { /* enough bytes on the wire ? */
 		ND_PRINT((ndo, "|OSI"));
 		return;
 	}
 
 	if (ndo->ndo_eflag)
@@ -683,20 +682,20 @@ isoclns_print(netdissect_options *ndo,
 
 	switch (*p) {
 
 	case NLPID_CLNP:
 		if (!clnp_print(ndo, p, length))
-			print_unknown_data(ndo, p, "\n\t", caplen);
+			print_unknown_data(ndo, p, "\n\t", length);
 		break;
 
 	case NLPID_ESIS:
 		esis_print(ndo, p, length);
 		return;
 
 	case NLPID_ISIS:
 		if (!isis_print(ndo, p, length))
-			print_unknown_data(ndo, p, "\n\t", caplen);
+			print_unknown_data(ndo, p, "\n\t", length);
 		break;
 
 	case NLPID_NULLNS:
 		ND_PRINT((ndo, "%slength: %u", ndo->ndo_eflag ? "" : ", ", length));
 		break;
@@ -719,12 +718,12 @@ isoclns_print(netdissect_options *ndo,
 
 	default:
 		if (!ndo->ndo_eflag)
 			ND_PRINT((ndo, "OSI NLPID 0x%02x unknown", *p));
 		ND_PRINT((ndo, "%slength: %u", ndo->ndo_eflag ? "" : ", ", length));
-		if (caplen > 1)
-			print_unknown_data(ndo, p, "\n\t", caplen);
+		if (length > 1)
+			print_unknown_data(ndo, p, "\n\t", length);
 		break;
 	}
 }
 
 #define	CLNP_PDU_ER	 1
diff --git a/print-juniper.c b/print-juniper.c
index e4ee11c0b..9a258d9ae 100644
--- a/print-juniper.c
+++ b/print-juniper.c
@@ -791,11 +791,11 @@ juniper_mlppp_print(netdissect_options *ndo,
             return l2info.header_len;
         case JUNIPER_LSQ_L3_PROTO_MPLS:
             mpls_print(ndo, p, l2info.length);
             return l2info.header_len;
         case JUNIPER_LSQ_L3_PROTO_ISO:
-            isoclns_print(ndo, p, l2info.length, l2info.caplen);
+            isoclns_print(ndo, p, l2info.length);
             return l2info.header_len;
         default:
             break;
         }
 
@@ -846,11 +846,11 @@ juniper_mfr_print(netdissect_options *ndo,
                 return l2info.header_len;
             case JUNIPER_LSQ_L3_PROTO_MPLS:
                 mpls_print(ndo, p, l2info.length);
                 return l2info.header_len;
             case JUNIPER_LSQ_L3_PROTO_ISO:
-                isoclns_print(ndo, p, l2info.length, l2info.caplen);
+                isoclns_print(ndo, p, l2info.length);
                 return l2info.header_len;
             default:
                 break;
             }
             return l2info.header_len;
@@ -859,17 +859,17 @@ juniper_mfr_print(netdissect_options *ndo,
         /* suppress Bundle-ID if frame was captured on a child-link */
         if (ndo->ndo_eflag && EXTRACT_32BITS(l2info.cookie) != 1)
             ND_PRINT((ndo, "Bundle-ID %u, ", l2info.bundle));
         switch (l2info.proto) {
         case (LLCSAP_ISONS<<8 | LLCSAP_ISONS):
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             break;
         case (LLC_UI<<8 | NLPID_Q933):
         case (LLC_UI<<8 | NLPID_IP):
         case (LLC_UI<<8 | NLPID_IP6):
             /* pass IP{4,6} to the OSI layer for proper link-layer printing */
-            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
+            isoclns_print(ndo, p - 1, l2info.length + 1);
             break;
         default:
             ND_PRINT((ndo, "unknown protocol 0x%04x, length %u", l2info.proto, l2info.length));
         }
 
@@ -894,17 +894,17 @@ juniper_mlfr_print(netdissect_options *ndo,
         if (ndo->ndo_eflag && EXTRACT_32BITS(l2info.cookie) != 1)
             ND_PRINT((ndo, "Bundle-ID %u, ", l2info.bundle));
         switch (l2info.proto) {
         case (LLC_UI):
         case (LLC_UI<<8):
-            isoclns_print(ndo, p, l2info.length, l2info.caplen);
+            isoclns_print(ndo, p, l2info.length);
             break;
         case (LLC_UI<<8 | NLPID_Q933):
         case (LLC_UI<<8 | NLPID_IP):
         case (LLC_UI<<8 | NLPID_IP6):
             /* pass IP{4,6} to the OSI layer for proper link-layer printing */
-            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
+            isoclns_print(ndo, p - 1, l2info.length + 1);
             break;
         default:
             ND_PRINT((ndo, "unknown protocol 0x%04x, length %u", l2info.proto, l2info.length));
         }
 
@@ -947,11 +947,11 @@ juniper_atm1_print(netdissect_options *ndo,
             if (llc_hdrlen > 0)
                 return l2info.header_len;
         }
 
         if (p[0] == 0x03) { /* Cisco style NLPID encaps ? */
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             /* FIXME check if frame was recognized */
             return l2info.header_len;
         }
 
         if (ip_heuristic_guess(ndo, p, l2info.length) != 0) /* last try - vcmux encaps ? */
@@ -1002,11 +1002,11 @@ juniper_atm2_print(netdissect_options *ndo,
             ether_print(ndo, p, l2info.length, l2info.caplen, NULL, NULL);
             return l2info.header_len;
         }
 
         if (p[0] == 0x03) { /* Cisco style NLPID encaps ? */
-            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
+            isoclns_print(ndo, p + 1, l2info.length - 1);
             /* FIXME check if frame was recognized */
             return l2info.header_len;
         }
 
         if(juniper_ppp_heuristic_guess(ndo, p, l2info.length) != 0) /* PPPoA vcmux encaps ? */
diff --git a/print-llc.c b/print-llc.c
index 6bdf59984..be8886abd 100644
--- a/print-llc.c
+++ b/print-llc.c
@@ -322,11 +322,11 @@ llc_print(netdissect_options *ndo, const u_char *p, u_int length, u_int caplen,
 		return (hdrlen);
 	}
 #endif
 	if (ssap == LLCSAP_ISONS && dsap == LLCSAP_ISONS
 	    && control == LLC_UI) {
-		isoclns_print(ndo, p, length, caplen);
+		isoclns_print(ndo, p, length);
 		return (hdrlen);
 	}
 
 	if (!ndo->ndo_eflag) {
 		if (ssap == dsap) {
diff --git a/print-mpls.c b/print-mpls.c
index ba4223346..5c26e4f9b 100644
--- a/print-mpls.c
+++ b/print-mpls.c
@@ -199,11 +199,11 @@ mpls_print(netdissect_options *ndo, const u_char *bp, u_int length)
 	case PT_IPV6:
 		ip6_print(ndo, p, length);
 		break;
 
 	case PT_OSI:
-		isoclns_print(ndo, p, length, length);
+		isoclns_print(ndo, p, length);
 		break;
 
 	default:
 		break;
 	}
diff --git a/print-null.c b/print-null.c
index 14df5fd15..59b691cc6 100644
--- a/print-null.c
+++ b/print-null.c
@@ -115,11 +115,11 @@ null_if_print(netdissect_options *ndo, const struct pcap_pkthdr *h, const u_char
 	case BSD_AFNUM_INET6_DARWIN:
 		ip6_print(ndo, p, length);
 		break;
 
 	case BSD_AFNUM_ISO:
-		isoclns_print(ndo, p, length, caplen);
+		isoclns_print(ndo, p, length);
 		break;
 
 	case BSD_AFNUM_APPLETALK:
 		atalk_print(ndo, p, length);
 		break;
diff --git a/print-ppp.c b/print-ppp.c
index b30f224c8..134389209 100644
--- a/print-ppp.c
+++ b/print-ppp.c
@@ -1482,11 +1482,11 @@ handle_ppp(netdissect_options *ndo,
 	case ETHERTYPE_IPX:	/*XXX*/
 	case PPP_IPX:
 		ipx_print(ndo, p, length);
 		break;
 	case PPP_OSI:
-		isoclns_print(ndo, p, length, length);
+		isoclns_print(ndo, p, length);
 		break;
 	case PPP_MPLS_UCAST:
 	case PPP_MPLS_MCAST:
 		mpls_print(ndo, p, length);
 		break;
diff --git a/tests/TESTLIST b/tests/TESTLIST
index ee6fb46ea..986c18fe8 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -440,10 +440,11 @@ isoclns-heapoverflow-2	isoclns-heapoverflow-2.pcap	isoclns-heapoverflow-2.out	-e
 isoclns-heapoverflow-3	isoclns-heapoverflow-3.pcap	isoclns-heapoverflow-3.out	-e -c1
 stp-v4-length-sigsegv	stp-v4-length-sigsegv.pcap	stp-v4-length-sigsegv.out
 hoobr_pimv1		hoobr_pimv1.pcap		hoobr_pimv1.out
 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
 
 # bad packets from Wilfried Kirsch
 slip-bad-direction	slip-bad-direction.pcap		slip-bad-direction.out	-ve
 
 # RTP tests
diff --git a/tests/isoclns-oobr.out b/tests/isoclns-oobr.out
new file mode 100644
index 000000000..c2cfdfb23
--- /dev/null
+++ b/tests/isoclns-oobr.out
@@ -0,0 +1 @@
+|OSI
diff --git a/tests/isoclns-oobr.pcap b/tests/isoclns-oobr.pcap
new file mode 100644
index 000000000..276b8a921
Binary files /dev/null and b/tests/isoclns-oobr.pcap differ

Vulnerable code: extern void isoclns_print(netdissect_options *, const u_char *, u_int, u_int);
            isoclns_print(ndo, p + 1, length - 1, caplen - 1);
                    isoclns_print(ndo, p + 1, length - 1, ndo->ndo_snapend - p - 1);
                    isoclns_print(ndo, p, length, ndo->ndo_snapend - p);
		isoclns_print(ndo, p + 1, length - 1, caplen - 1);
		isoclns_print(ndo, p - 1, length + 1, ndo->ndo_snapend - p + 1); /* OSI printers need the NLPID field */
		isoclns_print(ndo, bp, len, ndo->ndo_snapend - bp);
isoclns_print(netdissect_options *ndo,
              const uint8_t *p, u_int length, u_int caplen)
	if (caplen <= 1) { /* enough bytes on the wire ? */
			print_unknown_data(ndo, p, "\n\t", caplen);
			print_unknown_data(ndo, p, "\n\t", caplen);
		if (caplen > 1)
			print_unknown_data(ndo, p, "\n\t", caplen);
            isoclns_print(ndo, p, l2info.length, l2info.caplen);
                isoclns_print(ndo, p, l2info.length, l2info.caplen);
            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
            isoclns_print(ndo, p, l2info.length, l2info.caplen);
            isoclns_print(ndo, p - 1, l2info.length + 1, l2info.caplen + 1);
            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
            isoclns_print(ndo, p + 1, l2info.length - 1, l2info.caplen - 1);
		isoclns_print(ndo, p, length, caplen);
		isoclns_print(ndo, p, length, length);
		isoclns_print(ndo, p, length, caplen);
		isoclns_print(ndo, p, length, length);
Fixed code: extern void isoclns_print(netdissect_options *, const u_char *, u_int);
            isoclns_print(ndo, p + 1, length - 1);
                    isoclns_print(ndo, p + 1, length - 1);
                    isoclns_print(ndo, p, length);
		isoclns_print(ndo, p + 1, length - 1);
		isoclns_print(ndo, p - 1, length + 1); /* OSI printers need the NLPID field */
		isoclns_print(ndo, bp, len);
isoclns_print(netdissect_options *ndo, const uint8_t *p, u_int length)
	if (!ND_TTEST(*p)) { /* enough bytes on the wire ? */
			print_unknown_data(ndo, p, "\n\t", length);
			print_unknown_data(ndo, p, "\n\t", length);
		if (length > 1)
			print_unknown_data(ndo, p, "\n\t", length);
            isoclns_print(ndo, p, l2info.length);
                isoclns_print(ndo, p, l2info.length);
            isoclns_print(ndo, p + 1, l2info.length - 1);
            isoclns_print(ndo, p - 1, l2info.length + 1);
            isoclns_print(ndo, p, l2info.length);
            isoclns_print(ndo, p - 1, l2info.length + 1);
            isoclns_print(ndo, p + 1, l2info.length - 1);
            isoclns_print(ndo, p + 1, l2info.length - 1);
		isoclns_print(ndo, p, length);
		isoclns_print(ndo, p, length);
		isoclns_print(ndo, p, length);
		isoclns_print(ndo, p, length);
isoclns-oobr		isoclns-oobr.pcap		isoclns-oobr.out
|OSI