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 --git a/print-zephyr.c b/print-zephyr.c
index eb7e382bc54491b50026dad383b4f58ee87ea6ce..735e273f0770bff33fa1dc962a497eb5514bae1e 100644 (file)
--- a/print-zephyr.c
+++ b/print-zephyr.c
@@ -83,24 +83,34 @@ static const struct tok z_types[] = {
 static char z_buf[256];
 
 static const char *
 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;
 
 {
     const char *s;
 

-    if (*len <= 0 || !pptr || !*pptr)
-       return NULL;
-    if (*pptr > (const char *) ndo->ndo_snapend)
-       return NULL;
-
+    /* Start of string */

     s = *pptr;
     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)--;
     }
        (*pptr)++;
        (*len)--;
     }

+    /* Skip the NUL terminator */

     (*pptr)++;
     (*len)--;
     (*pptr)++;
     (*len)--;

-    if (*len < 0 || *pptr > (const char *) ndo->ndo_snapend)
-       return NULL;

     return s;
 }
 
     return s;
 }
 


@@ -139,6 +149,7 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     int parselen = length;
     const char *s;
     int lose = 0;
     int parselen = length;
     const char *s;
     int lose = 0;

+    int truncated = 0;

 
     /* squelch compiler warnings */
 
 
     /* squelch compiler warnings */
 


@@ -149,8 +160,9 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     z.sender = 0;
     z.recipient = 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)                 \
        if (!s) lose = 1;
 
 #define PARSE_FIELD_INT(field)                 \


@@ -183,10 +195,8 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     PARSE_FIELD_INT(z.multi);
     PARSE_FIELD_STR(z.multi_uid);
 
     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, " zephyr"));
     if (strncmp(z.version+4, "0.2", 3)) {


@@ -318,4 +328,9 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
     ND_PRINT((ndo, " to %s", z_triple(z.class, z.inst, z.recipient)));
     if (*z.opcode)
        ND_PRINT((ndo, " op %s", z.opcode));
     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 350f871eafe5813c0a1a910935cd312ed88b15d6..0829e90d2d1b496b23e8f622128b20235945e5c3 100644 (file)
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -462,6 +462,7 @@ 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
 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 tests
 # fuzzed pcap

diff --git a/tests/zephyr-oobr.out b/tests/zephyr-oobr.out
new file mode 100644 (file)
index 0000000..7f1ee1d
--- /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 (file)
index 0000000..af71c59
Binary files /dev/null and b/tests/zephyr-oobr.pcap differ