Skip to content

Commit d17507f

Browse files
guyharrisinfrastation
authored andcommitted
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).
1 parent de981e6 commit d17507f

File tree

4 files changed

+33
-15
lines changed

4 files changed

+33
-15
lines changed

Diff for: print-zephyr.c

+30-15
Original file line numberDiff line numberDiff line change
@@ -83,24 +83,34 @@ static const struct tok z_types[] = {
8383
static char z_buf[256];
8484

8585
static const char *
86-
parse_field(netdissect_options *ndo, const char **pptr, int *len)
86+
parse_field(netdissect_options *ndo, const char **pptr, int *len, int *truncated)
8787
{
8888
const char *s;
8989

90-
if (*len <= 0 || !pptr || !*pptr)
91-
return NULL;
92-
if (*pptr > (const char *) ndo->ndo_snapend)
93-
return NULL;
94-
90+
/* Start of string */
9591
s = *pptr;
96-
while (*pptr <= (const char *) ndo->ndo_snapend && *len >= 0 && **pptr) {
92+
/* Scan for the NUL terminator */
93+
for (;;) {
94+
if (*len == 0) {
95+
/* Ran out of packet data without finding it */
96+
return NULL;
97+
}
98+
if (!ND_TTEST(**pptr)) {
99+
/* Ran out of captured data without finding it */
100+
*truncated = 1;
101+
return NULL;
102+
}
103+
if (**pptr == '\0') {
104+
/* Found it */
105+
break;
106+
}
107+
/* Keep scanning */
97108
(*pptr)++;
98109
(*len)--;
99110
}
111+
/* Skip the NUL terminator */
100112
(*pptr)++;
101113
(*len)--;
102-
if (*len < 0 || *pptr > (const char *) ndo->ndo_snapend)
103-
return NULL;
104114
return s;
105115
}
106116

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

143154
/* squelch compiler warnings */
144155

@@ -149,8 +160,9 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
149160
z.sender = 0;
150161
z.recipient = 0;
151162

152-
#define PARSE_STRING \
153-
s = parse_field(ndo, &parse, &parselen); \
163+
#define PARSE_STRING \
164+
s = parse_field(ndo, &parse, &parselen, &truncated); \
165+
if (truncated) goto trunc; \
154166
if (!s) lose = 1;
155167

156168
#define PARSE_FIELD_INT(field) \
@@ -183,10 +195,8 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
183195
PARSE_FIELD_INT(z.multi);
184196
PARSE_FIELD_STR(z.multi_uid);
185197

186-
if (lose) {
187-
ND_PRINT((ndo, " [|zephyr] (%d)", length));
188-
return;
189-
}
198+
if (lose)
199+
goto trunc;
190200

191201
ND_PRINT((ndo, " zephyr"));
192202
if (strncmp(z.version+4, "0.2", 3)) {
@@ -318,4 +328,9 @@ zephyr_print(netdissect_options *ndo, const u_char *cp, int length)
318328
ND_PRINT((ndo, " to %s", z_triple(z.class, z.inst, z.recipient)));
319329
if (*z.opcode)
320330
ND_PRINT((ndo, " op %s", z.opcode));
331+
return;
332+
333+
trunc:
334+
ND_PRINT((ndo, " [|zephyr] (%d)", length));
335+
return;
321336
}

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,7 @@ icmp-cksum-oobr-4 icmp-cksum-oobr-4.pcap icmp-cksum-oobr-4.out -vvv -e
462462
tok2str-oobr-1 tok2str-oobr-1.pcap tok2str-oobr-1.out -vvv -e
463463
tok2str-oobr-2 tok2str-oobr-2.pcap tok2str-oobr-2.out -vvv -e
464464
eigrp-tlv-oobr eigrp-tlv-oobr.pcap eigrp-tlv-oobr.out -vvv -e
465+
zephyr-oobr zephyr-oobr.pcap zephyr-oobr.out -vvv -e
465466

466467
# RTP tests
467468
# fuzzed pcap

Diff for: tests/zephyr-oobr.out

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
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)
2+
167.155.6.190.2104 > 167.155.9.153.514: [udp sum ok]

Diff for: tests/zephyr-oobr.pcap

64 KB
Binary file not shown.

0 commit comments

Comments
 (0)