CVE-2017-13687: Out-of-bounds Read (CWE-125)
value: The Cisco HDLC parser in tcpdump before 4.9.2 has a buffer over-read in print-chdlc.c:chdlc_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: print-chdlc.c
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13687/CHDLC: Improve bounds and length checks.

Prevent a possible buffer overread in chdlc_print() and replace the
custom check in chdlc_if_print() with a standard check in chdlc_print()
so that the latter certainly does not over-read even when reached via
juniper_chdlc_print(). Add length checks.

Diff: diff --git a/print-chdlc.c b/print-chdlc.c
index 450d286848..ca96cc5060 100644
--- a/print-chdlc.c
+++ b/print-chdlc.c
@@ -44,25 +44,22 @@ static const struct tok chdlc_cast_values[] = {
 
 /* Standard CHDLC printer */
 u_int
 chdlc_if_print(netdissect_options *ndo, const struct pcap_pkthdr *h, register const u_char *p)
 {
-	register u_int length = h->len;
-	register u_int caplen = h->caplen;
-
-	if (caplen < CHDLC_HDRLEN) {
-		ND_PRINT((ndo, "[|chdlc]"));
-		return (caplen);
-	}
-        return (chdlc_print(ndo, p,length));
+	return chdlc_print(ndo, p, h->len);
 }
 
 u_int
 chdlc_print(netdissect_options *ndo, register const u_char *p, u_int length)
 {
 	u_int proto;
+	const u_char *bp = p;
 
+	if (length < CHDLC_HDRLEN)
+		goto trunc;
+	ND_TCHECK2(*p, CHDLC_HDRLEN);
 	proto = EXTRACT_16BITS(&p[2]);
 	if (ndo->ndo_eflag) {
                 ND_PRINT((ndo, "%s, ethertype %s (0x%04x), length %u: ",
                        tok2str(chdlc_cast_values, "0x%02x", p[0]),
                        tok2str(ethertype_values, "Unknown", proto),
@@ -92,10 +89,13 @@ chdlc_print(netdissect_options *ndo, register const u_char *p, u_int length)
         case ETHERTYPE_MPLS_MULTI:
                 mpls_print(ndo, p, length);
 		break;
         case ETHERTYPE_ISO:
                 /* is the fudge byte set ? lets verify by spotting ISO headers */
+                if (length < 2)
+                    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);
                 else
@@ -106,10 +106,14 @@ chdlc_print(netdissect_options *ndo, register const u_char *p, u_int length)
                         ND_PRINT((ndo, "unknown CHDLC protocol (0x%04x)", proto));
                 break;
 	}
 
 	return (CHDLC_HDRLEN);
+
+trunc:
+	ND_PRINT((ndo, "[|chdlc]"));
+	return ndo->ndo_snapend - bp;
 }
 
 /*
  * The fixed-length portion of a SLARP packet.
  */

Vulnerable code: 	register u_int length = h->len;
	register u_int caplen = h->caplen;

	if (caplen < CHDLC_HDRLEN) {
		ND_PRINT((ndo, "[|chdlc]"));
		return (caplen);
	}
        return (chdlc_print(ndo, p,length));
Fixed code: 	return chdlc_print(ndo, p, h->len);
	const u_char *bp = p;
	if (length < CHDLC_HDRLEN)
		goto trunc;
	ND_TCHECK2(*p, CHDLC_HDRLEN);
                if (length < 2)
                    goto trunc;
                ND_TCHECK_16BITS(p);

trunc:
	ND_PRINT((ndo, "[|chdlc]"));
	return ndo->ndo_snapend - bp;