Skip to content

Commit 4c3aee4

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-13040/MPTCP: Clean up printing DSS suboption.
Do the length checking inline; that means we print stuff up to the point at which we run out of option data. First check to make sure we have at least 4 bytes of option, so we have flags to check. This fixes a buffer over-read discovered by Kim Gwan Yeong. Add a test using the capture file supplied by the reporter(s).
1 parent e0a5a02 commit 4c3aee4

File tree

4 files changed

+55
-34
lines changed

4 files changed

+55
-34
lines changed

Diff for: print-mptcp.c

+50-34
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ mp_capable_print(netdissect_options *ndo,
178178
{
179179
const struct mp_capable *mpc = (const struct mp_capable *) opt;
180180

181-
if (!(opt_len == 12 && flags & TH_SYN) &&
181+
if (!(opt_len == 12 && (flags & TH_SYN)) &&
182182
!(opt_len == 20 && (flags & (TH_SYN | TH_ACK)) == TH_ACK))
183183
return 0;
184184

@@ -202,9 +202,9 @@ mp_join_print(netdissect_options *ndo,
202202
{
203203
const struct mp_join *mpj = (const struct mp_join *) opt;
204204

205-
if (!(opt_len == 12 && flags & TH_SYN) &&
205+
if (!(opt_len == 12 && (flags & TH_SYN)) &&
206206
!(opt_len == 16 && (flags & (TH_SYN | TH_ACK)) == (TH_SYN | TH_ACK)) &&
207-
!(opt_len == 24 && flags & TH_ACK))
207+
!(opt_len == 24 && (flags & TH_ACK)))
208208
return 0;
209209

210210
if (opt_len != 24) {
@@ -236,76 +236,92 @@ mp_join_print(netdissect_options *ndo,
236236
return 1;
237237
}
238238

239-
static u_int mp_dss_len(const struct mp_dss *m, int csum)
240-
{
241-
u_int len;
242-
243-
len = 4;
244-
if (m->flags & MP_DSS_A) {
245-
/* Ack present - 4 or 8 octets */
246-
len += (m->flags & MP_DSS_a) ? 8 : 4;
247-
}
248-
if (m->flags & MP_DSS_M) {
249-
/*
250-
* Data Sequence Number (DSN), Subflow Sequence Number (SSN),
251-
* Data-Level Length present, and Checksum possibly present.
252-
* All but the Checksum are 10 bytes if the m flag is
253-
* clear (4-byte DSN) and 14 bytes if the m flag is set
254-
* (8-byte DSN).
255-
*/
256-
len += (m->flags & MP_DSS_m) ? 14 : 10;
257-
258-
/*
259-
* The Checksum is present only if negotiated.
260-
*/
261-
if (csum)
262-
len += 2;
263-
}
264-
return len;
265-
}
266-
267239
static int
268240
mp_dss_print(netdissect_options *ndo,
269241
const u_char *opt, u_int opt_len, u_char flags)
270242
{
271243
const struct mp_dss *mdss = (const struct mp_dss *) opt;
272244

273-
if ((opt_len != mp_dss_len(mdss, 1) &&
274-
opt_len != mp_dss_len(mdss, 0)) || flags & TH_SYN)
245+
/* We need the flags, at a minimum. */
246+
if (opt_len < 4)
247+
return 0;
248+
249+
if (flags & TH_SYN)
275250
return 0;
276251

277252
if (mdss->flags & MP_DSS_F)
278253
ND_PRINT((ndo, " fin"));
279254

280255
opt += 4;
256+
opt_len -= 4;
281257
if (mdss->flags & MP_DSS_A) {
258+
/* Ack present */
282259
ND_PRINT((ndo, " ack "));
260+
/*
261+
* If the a flag is set, we have an 8-byte ack; if it's
262+
* clear, we have a 4-byte ack.
263+
*/
283264
if (mdss->flags & MP_DSS_a) {
265+
if (opt_len < 8)
266+
return 0;
284267
ND_PRINT((ndo, "%" PRIu64, EXTRACT_64BITS(opt)));
285268
opt += 8;
269+
opt_len -= 8;
286270
} else {
271+
if (opt_len < 4)
272+
return 0;
287273
ND_PRINT((ndo, "%u", EXTRACT_32BITS(opt)));
288274
opt += 4;
275+
opt_len -= 4;
289276
}
290277
}
291278

292279
if (mdss->flags & MP_DSS_M) {
280+
/*
281+
* Data Sequence Number (DSN), Subflow Sequence Number (SSN),
282+
* Data-Level Length present, and Checksum possibly present.
283+
*/
293284
ND_PRINT((ndo, " seq "));
285+
/*
286+
* If the m flag is set, we have an 8-byte NDS; if it's clear,
287+
* we have a 4-byte DSN.
288+
*/
294289
if (mdss->flags & MP_DSS_m) {
290+
if (opt_len < 8)
291+
return 0;
295292
ND_PRINT((ndo, "%" PRIu64, EXTRACT_64BITS(opt)));
296293
opt += 8;
294+
opt_len -= 8;
297295
} else {
296+
if (opt_len < 4)
297+
return 0;
298298
ND_PRINT((ndo, "%u", EXTRACT_32BITS(opt)));
299299
opt += 4;
300+
opt_len -= 4;
300301
}
302+
if (opt_len < 4)
303+
return 0;
301304
ND_PRINT((ndo, " subseq %u", EXTRACT_32BITS(opt)));
302305
opt += 4;
306+
opt_len -= 4;
307+
if (opt_len < 2)
308+
return 0;
303309
ND_PRINT((ndo, " len %u", EXTRACT_16BITS(opt)));
304310
opt += 2;
311+
opt_len -= 2;
305312

306-
if (opt_len == mp_dss_len(mdss, 1))
313+
/*
314+
* The Checksum is present only if negotiated.
315+
* If there are at least 2 bytes left, process the next 2
316+
* bytes as the Checksum.
317+
*/
318+
if (opt_len >= 2) {
307319
ND_PRINT((ndo, " csum 0x%x", EXTRACT_16BITS(opt)));
320+
opt_len -= 2;
321+
}
308322
}
323+
if (opt_len != 0)
324+
return 0;
309325
return 1;
310326
}
311327

Diff for: tests/TESTLIST

+3
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,9 @@ isakmpv1-attr-oobr isakmpv1-attr-oobr.pcap isakmpv1-attr-oobr.out -v
558558
# bad packets from Katie Holly
559559
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out
560560

561+
# bad packets from Kim Gwan Yeong
562+
mptcp-dss-oobr mptcp-dss-oobr.pcap mptcp-dss-oobr.out -v
563+
561564
# RTP tests
562565
# fuzzed pcap
563566
rtp-seg-fault-1 rtp-seg-fault-1.pcap rtp-seg-fault-1.out -v -T rtp

Diff for: tests/mptcp-dss-oobr.out

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
IP (tos 0x10, ttl 64, id 39991, offset 0, flags [DF], proto TCP (6), length 60)
2+
127.0.0.1.57370 > 127.0.0.1.23: Flags [S], seq 1736820995, win 32792, options [mss 16396,sackOK,TS val 597120308 ecr 0,mptcp dss[bad opt]>

Diff for: tests/mptcp-dss-oobr.pcap

114 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)