Skip to content

Commit 7029d15

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-13029/PPP: Fix a bounds check, and clean up other bounds checks.
For configuration protocol options, use ND_TCHECK() and ND_TCHECK_nBITS() macros, passing them the appropriate pointer argument. This fixes one case where the ND_TCHECK2() call they replace was not checking enough bytes. This fixes a buffer over-read discovered by Bhargava Shastry, SecT/TU Berlin. Add a test using the capture file supplied by the reporter(s), modified so the capture file won't be rejected as an invalid capture.
1 parent 29e5470 commit 7029d15

4 files changed

+17
-13
lines changed

Diff for: print-ppp.c

+13-13
Original file line numberDiff line numberDiff line change
@@ -611,7 +611,7 @@ print_lcp_config_options(netdissect_options *ndo,
611611
ND_PRINT((ndo, " (length bogus, should be >= 6)"));
612612
return len;
613613
}
614-
ND_TCHECK2(*(p + 2), 3);
614+
ND_TCHECK_24BITS(p + 2);
615615
ND_PRINT((ndo, ": Vendor: %s (%u)",
616616
tok2str(oui_values,"Unknown",EXTRACT_24BITS(p+2)),
617617
EXTRACT_24BITS(p + 2)));
@@ -630,23 +630,23 @@ print_lcp_config_options(netdissect_options *ndo,
630630
ND_PRINT((ndo, " (length bogus, should be = 4)"));
631631
return len;
632632
}
633-
ND_TCHECK2(*(p + 2), 2);
633+
ND_TCHECK_16BITS(p + 2);
634634
ND_PRINT((ndo, ": %u", EXTRACT_16BITS(p + 2)));
635635
break;
636636
case LCPOPT_ACCM:
637637
if (len != 6) {
638638
ND_PRINT((ndo, " (length bogus, should be = 6)"));
639639
return len;
640640
}
641-
ND_TCHECK2(*(p + 2), 4);
641+
ND_TCHECK_32BITS(p + 2);
642642
ND_PRINT((ndo, ": 0x%08x", EXTRACT_32BITS(p + 2)));
643643
break;
644644
case LCPOPT_AP:
645645
if (len < 4) {
646646
ND_PRINT((ndo, " (length bogus, should be >= 4)"));
647647
return len;
648648
}
649-
ND_TCHECK2(*(p + 2), 2);
649+
ND_TCHECK_16BITS(p + 2);
650650
ND_PRINT((ndo, ": %s", tok2str(ppptype2str, "Unknown Auth Proto (0x04x)", EXTRACT_16BITS(p + 2))));
651651

652652
switch (EXTRACT_16BITS(p+2)) {
@@ -668,7 +668,7 @@ print_lcp_config_options(netdissect_options *ndo,
668668
ND_PRINT((ndo, " (length bogus, should be >= 4)"));
669669
return 0;
670670
}
671-
ND_TCHECK2(*(p + 2), 2);
671+
ND_TCHECK_16BITS(p+2);
672672
if (EXTRACT_16BITS(p+2) == PPP_LQM)
673673
ND_PRINT((ndo, ": LQR"));
674674
else
@@ -679,7 +679,7 @@ print_lcp_config_options(netdissect_options *ndo,
679679
ND_PRINT((ndo, " (length bogus, should be = 6)"));
680680
return 0;
681681
}
682-
ND_TCHECK2(*(p + 2), 4);
682+
ND_TCHECK_32BITS(p + 2);
683683
ND_PRINT((ndo, ": 0x%08x", EXTRACT_32BITS(p + 2)));
684684
break;
685685
case LCPOPT_PFC:
@@ -691,7 +691,7 @@ print_lcp_config_options(netdissect_options *ndo,
691691
ND_PRINT((ndo, " (length bogus, should be = 4)"));
692692
return 0;
693693
}
694-
ND_TCHECK2(*(p + 2), 2);
694+
ND_TCHECK_16BITS(p + 2);
695695
ND_PRINT((ndo, ": 0x%04x", EXTRACT_16BITS(p + 2)));
696696
break;
697697
case LCPOPT_CBACK:
@@ -710,7 +710,7 @@ print_lcp_config_options(netdissect_options *ndo,
710710
ND_PRINT((ndo, " (length bogus, should be = 4)"));
711711
return 0;
712712
}
713-
ND_TCHECK2(*(p + 2), 2);
713+
ND_TCHECK_16BITS(p + 2);
714714
ND_PRINT((ndo, ": %u", EXTRACT_16BITS(p + 2)));
715715
break;
716716
case LCPOPT_MLED:
@@ -1055,7 +1055,7 @@ print_ipcp_config_options(netdissect_options *ndo,
10551055
ND_PRINT((ndo, " (length bogus, should be >= 4)"));
10561056
return 0;
10571057
}
1058-
ND_TCHECK2(*(p + 2), 2);
1058+
ND_TCHECK_16BITS(p+2);
10591059
compproto = EXTRACT_16BITS(p+2);
10601060

10611061
ND_PRINT((ndo, ": %s (0x%02x):",
@@ -1241,7 +1241,7 @@ print_ccp_config_options(netdissect_options *ndo,
12411241
ND_PRINT((ndo, " (length bogus, should be >= 3)"));
12421242
return len;
12431243
}
1244-
ND_TCHECK2(*(p + 2), 1);
1244+
ND_TCHECK(p[2]);
12451245
ND_PRINT((ndo, ": Version: %u, Dictionary Bits: %u",
12461246
p[2] >> 5, p[2] & 0x1f));
12471247
break;
@@ -1250,7 +1250,7 @@ print_ccp_config_options(netdissect_options *ndo,
12501250
ND_PRINT((ndo, " (length bogus, should be >= 4)"));
12511251
return len;
12521252
}
1253-
ND_TCHECK2(*(p + 2), 1);
1253+
ND_TCHECK(p[3]);
12541254
ND_PRINT((ndo, ": Features: %u, PxP: %s, History: %u, #CTX-ID: %u",
12551255
(p[2] & 0xc0) >> 6,
12561256
(p[2] & 0x20) ? "Enabled" : "Disabled",
@@ -1261,7 +1261,7 @@ print_ccp_config_options(netdissect_options *ndo,
12611261
ND_PRINT((ndo, " (length bogus, should be >= 4)"));
12621262
return len;
12631263
}
1264-
ND_TCHECK2(*(p + 2), 1);
1264+
ND_TCHECK(p[3]);
12651265
ND_PRINT((ndo, ": Window: %uK, Method: %s (0x%x), MBZ: %u, CHK: %u",
12661266
(p[2] & 0xf0) >> 4,
12671267
((p[2] & 0x0f) == 8) ? "zlib" : "unknown",
@@ -1336,7 +1336,7 @@ print_bacp_config_options(netdissect_options *ndo,
13361336
ND_PRINT((ndo, " (length bogus, should be = 6)"));
13371337
return len;
13381338
}
1339-
ND_TCHECK2(*(p + 2), 4);
1339+
ND_TCHECK_32BITS(p + 2);
13401340
ND_PRINT((ndo, ": Magic-Num 0x%08x", EXTRACT_32BITS(p + 2)));
13411341
break;
13421342
default:

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,7 @@ isis_stlv_asan-3 isis_stlv_asan-3.pcap isis_stlv_asan-3.out -v
534534
isis_stlv_asan-4 isis_stlv_asan-4.pcap isis_stlv_asan-4.out -v
535535
lldp_mgmt_addr_tlv_asan lldp_mgmt_addr_tlv_asan.pcap lldp_mgmt_addr_tlv_asan.out -v
536536
bootp_asan bootp_asan.pcap bootp_asan.out -v
537+
ppp_ccp_config_deflate_option_asan ppp_ccp_config_deflate_option_asan.pcap ppp_ccp_config_deflate_option_asan.out -v
537538

538539
# RTP tests
539540
# fuzzed pcap

Diff for: tests/ppp_ccp_config_deflate_option_asan.out

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
: CCP, Conf-Request (0x01), id 223, length 125685
2+
encoded length 15 (=Option(s) length 11)
3+
MVRCA Option (0x18), length 5[|ccp]

Diff for: tests/ppp_ccp_config_deflate_option_asan.pcap

63 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)