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

Use ND_TTEST() rather than comparing against ndo->ndo_snapend ourselves;
it's easy to get the tests wrong.

Check for running out of packet data before checking for running out of
captured data, and distinguish between running out of packet data (which
might just mean "no more strings") and running out of captured data
(which means "truncated").

This fixes a buffer over-read discovered by Forcepoint's security
researchers Otto Airamo & Antti Levomäki.

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

Diff: diff --git a/print-zephyr.c b/print-zephyr.c
index eb7e382bc..735e273f0 100644
--- a/print-zephyr.c
+++ b/print-zephyr.c
@@ -81,28 +81,38 @@ static const struct tok z_types[] = {
 };
 
 static char z_buf[256];
 
 static const char *
-parse_field(netdissect_options *ndo, const char **pptr, int *len)
+parse_field(netdissect_options *ndo, const char **pptr, int *len, int *truncated)
 {
     const char *s;
 
-    if (*len <= 0 || !pptr || !*pptr)
-	return NULL;
-    if (*pptr > (const char *) ndo->ndo_snapend)
-	return NULL;
-
+    /* Start of string */
     s = *pptr;
-    while (*pptr <= (const char *) ndo->ndo_snapend && *len >= 0 && **pptr) {
+    /* Scan for the NUL terminator */
+    for (;;) {
+	if (*len == 0) {
+	    /* Ran out of packet data without finding it */
+	    return NULL;
+	}
+	if (!ND_TTEST(**pptr)) {
+	    /* Ran out of captured data without finding it */
+	    *truncated = 1;
+	    return NULL;
+	}
+	if (**pptr == '\0') {
+	    /* Found it */
+	    break;
+	}
+	/* Keep scanning */
 	(*pptr)++;
 	(*len)--;
     }
+    /* Skip the NUL terminator */
     (*pptr)++;
     (*len)--;
-    if (*len < 0 || *pptr > (const char *) ndo->ndo_snapend)
-	return NULL;
     return s;
 }
 
 static const char *
 z_triple(const char *class, const char *inst, const char *recipient)
@@ -137,22 +147,24 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     struct z_packet z;
     const char *parse = (const char *) cp;
     int parselen = length;
     const char *s;
     int lose = 0;
+    int truncated = 0;
 
     /* squelch compiler warnings */
 
     z.kind = 0;
     z.class = 0;
     z.inst = 0;
     z.opcode = 0;
     z.sender = 0;
     z.recipient = 0;
 
-#define PARSE_STRING				\
-	s = parse_field(ndo, &parse, &parselen);	\
+#define PARSE_STRING						\
+	s = parse_field(ndo, &parse, &parselen, &truncated);	\
+	if (truncated) goto trunc;				\
 	if (!s) lose = 1;
 
 #define PARSE_FIELD_INT(field)			\
 	PARSE_STRING				\
 	if (!lose) field = strtol(s, 0, 16);
@@ -181,14 +193,12 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     PARSE_FIELD_STR(z.format);
     PARSE_FIELD_INT(z.cksum);
     PARSE_FIELD_INT(z.multi);
     PARSE_FIELD_STR(z.multi_uid);
 
-    if (lose) {
-	ND_PRINT((ndo, " [|zephyr] (%d)", length));
-	return;
-    }
+    if (lose)
+        goto trunc;
 
     ND_PRINT((ndo, " zephyr"));
     if (strncmp(z.version+4, "0.2", 3)) {
 	ND_PRINT((ndo, " v%s", z.version+4));
 	return;
@@ -316,6 +326,11 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
 	z.recipient = "*";
 
     ND_PRINT((ndo, " to %s", z_triple(z.class, z.inst, z.recipient)));
     if (*z.opcode)
 	ND_PRINT((ndo, " op %s", z.opcode));
+    return;
+
+trunc:
+    ND_PRINT((ndo, " [|zephyr] (%d)", length));
+    return;
 }
diff --git a/tests/TESTLIST b/tests/TESTLIST
index 350f871ea..0829e90d2 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -460,10 +460,11 @@ icmp-cksum-oobr-2	icmp-cksum-oobr-2.pcap		icmp-cksum-oobr-2.out	-vvv -e
 icmp-cksum-oobr-3	icmp-cksum-oobr-3.pcap		icmp-cksum-oobr-3.out	-vvv -e
 icmp-cksum-oobr-4	icmp-cksum-oobr-4.pcap		icmp-cksum-oobr-4.out	-vvv -e
 tok2str-oobr-1		tok2str-oobr-1.pcap		tok2str-oobr-1.out	-vvv -e
 tok2str-oobr-2		tok2str-oobr-2.pcap		tok2str-oobr-2.out	-vvv -e
 eigrp-tlv-oobr		eigrp-tlv-oobr.pcap		eigrp-tlv-oobr.out	-vvv -e
+zephyr-oobr		zephyr-oobr.pcap		zephyr-oobr.out		-vvv -e
 
 # 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/zephyr-oobr.out b/tests/zephyr-oobr.out
new file mode 100644
index 000000000..7f1ee1dfe
--- /dev/null
+++ b/tests/zephyr-oobr.out
@@ -0,0 +1,2 @@
+00:16:ca:92:12:01 > 00:15:e8:97:b2:01, ethertype IPv4 (0x0800), length 65535: (tos 0x0, ttl 124, id 16059, offset 0, flags [none], proto UDP (17), length 65521)
+    167.155.6.190.2104 > 167.155.9.153.514: [udp sum ok] 
diff --git a/tests/zephyr-oobr.pcap b/tests/zephyr-oobr.pcap
new file mode 100644
index 000000000..af71c594b
Binary files /dev/null and b/tests/zephyr-oobr.pcap differ

Vulnerable code: parse_field(netdissect_options *ndo, const char **pptr, int *len)
    if (*len <= 0 || !pptr || !*pptr)
	return NULL;
    if (*pptr > (const char *) ndo->ndo_snapend)
	return NULL;

    while (*pptr <= (const char *) ndo->ndo_snapend && *len >= 0 && **pptr) {
    if (*len < 0 || *pptr > (const char *) ndo->ndo_snapend)
	return NULL;
#define PARSE_STRING				\
	s = parse_field(ndo, &parse, &parselen);	\
    if (lose) {
	ND_PRINT((ndo, " [|zephyr] (%d)", length));
	return;
    }
Fixed code: parse_field(netdissect_options *ndo, const char **pptr, int *len, int *truncated)
    /* Start of string */
    /* Scan for the NUL terminator */
    for (;;) {
	if (*len == 0) {
	    /* Ran out of packet data without finding it */
	    return NULL;
	}
	if (!ND_TTEST(**pptr)) {
	    /* Ran out of captured data without finding it */
	    *truncated = 1;
	    return NULL;
	}
	if (**pptr == '\0') {
	    /* Found it */
	    break;
	}
	/* Keep scanning */
    /* Skip the NUL terminator */
    int truncated = 0;
#define PARSE_STRING						\
	s = parse_field(ndo, &parse, &parselen, &truncated);	\
	if (truncated) goto trunc;				\
    if (lose)
        goto trunc;
    return;

trunc:
    ND_PRINT((ndo, " [|zephyr] (%d)", length));
    return;
zephyr-oobr		zephyr-oobr.pcap		zephyr-oobr.out		-vvv -e
00:16:ca:92:12:01 > 00:15:e8:97:b2:01, ethertype IPv4 (0x0800), length 65535: (tos 0x0, ttl 124, id 16059, offset 0, flags [none], proto UDP (17), length 65521)
    167.155.6.190.2104 > 167.155.9.153.514: [udp sum ok]