Skip to content

Commit e0a5a02

Browse files
guyharrisinfrastation
authored andcommitted
CVE-2017-13039/IKEv1: Do more bounds checking.
Have ikev1_attrmap_print() and ikev1_attr_print() do full bounds checking, and return null on a bounds overflow. Have their callers check for a null return. 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 7335163 commit e0a5a02

File tree

4 files changed

+62
-25
lines changed

4 files changed

+62
-25
lines changed

Diff for: print-isakmp.c

+58-25
Original file line numberDiff line numberDiff line change
@@ -912,21 +912,25 @@ struct attrmap {
912912

913913
static const u_char *
914914
ikev1_attrmap_print(netdissect_options *ndo,
915-
const u_char *p, const u_char *ep,
915+
const u_char *p, const u_char *ep2,
916916
const struct attrmap *map, size_t nmap)
917917
{
918918
int totlen;
919919
uint32_t t, v;
920920

921+
ND_TCHECK(p[0]);
921922
if (p[0] & 0x80)
922923
totlen = 4;
923-
else
924+
else {
925+
ND_TCHECK_16BITS(&p[2]);
924926
totlen = 4 + EXTRACT_16BITS(&p[2]);
925-
if (ep < p + totlen) {
927+
}
928+
if (ep2 < p + totlen) {
926929
ND_PRINT((ndo,"[|attr]"));
927-
return ep + 1;
930+
return ep2 + 1;
928931
}
929932

933+
ND_TCHECK_16BITS(&p[0]);
930934
ND_PRINT((ndo,"("));
931935
t = EXTRACT_16BITS(&p[0]) & 0x7fff;
932936
if (map && t < nmap && map[t].type)
@@ -935,47 +939,71 @@ ikev1_attrmap_print(netdissect_options *ndo,
935939
ND_PRINT((ndo,"type=#%d ", t));
936940
if (p[0] & 0x80) {
937941
ND_PRINT((ndo,"value="));
942+
ND_TCHECK_16BITS(&p[2]);
938943
v = EXTRACT_16BITS(&p[2]);
939944
if (map && t < nmap && v < map[t].nvalue && map[t].value[v])
940945
ND_PRINT((ndo,"%s", map[t].value[v]));
941-
else
942-
rawprint(ndo, (const uint8_t *)&p[2], 2);
946+
else {
947+
if (!rawprint(ndo, (const uint8_t *)&p[2], 2)) {
948+
ND_PRINT((ndo,")"));
949+
goto trunc;
950+
}
951+
}
943952
} else {
944-
ND_PRINT((ndo,"len=%d value=", EXTRACT_16BITS(&p[2])));
945-
rawprint(ndo, (const uint8_t *)&p[4], EXTRACT_16BITS(&p[2]));
953+
ND_PRINT((ndo,"len=%d value=", totlen - 4));
954+
if (!rawprint(ndo, (const uint8_t *)&p[4], totlen - 4)) {
955+
ND_PRINT((ndo,")"));
956+
goto trunc;
957+
}
946958
}
947959
ND_PRINT((ndo,")"));
948960
return p + totlen;
961+
962+
trunc:
963+
return NULL;
949964
}
950965

951966
static const u_char *
952-
ikev1_attr_print(netdissect_options *ndo, const u_char *p, const u_char *ep)
967+
ikev1_attr_print(netdissect_options *ndo, const u_char *p, const u_char *ep2)
953968
{
954969
int totlen;
955970
uint32_t t;
956971

972+
ND_TCHECK(p[0]);
957973
if (p[0] & 0x80)
958974
totlen = 4;
959-
else
975+
else {
976+
ND_TCHECK_16BITS(&p[2]);
960977
totlen = 4 + EXTRACT_16BITS(&p[2]);
961-
if (ep < p + totlen) {
978+
}
979+
if (ep2 < p + totlen) {
962980
ND_PRINT((ndo,"[|attr]"));
963-
return ep + 1;
981+
return ep2 + 1;
964982
}
965983

984+
ND_TCHECK_16BITS(&p[0]);
966985
ND_PRINT((ndo,"("));
967986
t = EXTRACT_16BITS(&p[0]) & 0x7fff;
968987
ND_PRINT((ndo,"type=#%d ", t));
969988
if (p[0] & 0x80) {
970989
ND_PRINT((ndo,"value="));
971990
t = p[2];
972-
rawprint(ndo, (const uint8_t *)&p[2], 2);
991+
if (!rawprint(ndo, (const uint8_t *)&p[2], 2)) {
992+
ND_PRINT((ndo,")"));
993+
goto trunc;
994+
}
973995
} else {
974-
ND_PRINT((ndo,"len=%d value=", EXTRACT_16BITS(&p[2])));
975-
rawprint(ndo, (const uint8_t *)&p[4], EXTRACT_16BITS(&p[2]));
996+
ND_PRINT((ndo,"len=%d value=", totlen - 4));
997+
if (!rawprint(ndo, (const uint8_t *)&p[4], totlen - 4)) {
998+
ND_PRINT((ndo,")"));
999+
goto trunc;
1000+
}
9761001
}
9771002
ND_PRINT((ndo,")"));
9781003
return p + totlen;
1004+
1005+
trunc:
1006+
return NULL;
9791007
}
9801008

9811009
static const u_char *
@@ -1256,11 +1284,12 @@ ikev1_t_print(netdissect_options *ndo, u_char tpay _U_,
12561284
cp = (const u_char *)(p + 1);
12571285
ep2 = (const u_char *)p + item_len;
12581286
while (cp < ep && cp < ep2) {
1259-
if (map && nmap) {
1260-
cp = ikev1_attrmap_print(ndo, cp, (ep < ep2) ? ep : ep2,
1261-
map, nmap);
1262-
} else
1263-
cp = ikev1_attr_print(ndo, cp, (ep < ep2) ? ep : ep2);
1287+
if (map && nmap)
1288+
cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap);
1289+
else
1290+
cp = ikev1_attr_print(ndo, cp, ep2);
1291+
if (cp == NULL)
1292+
goto trunc;
12641293
}
12651294
if (ep < ep2)
12661295
ND_PRINT((ndo,"..."));
@@ -1724,8 +1753,11 @@ ikev1_n_print(netdissect_options *ndo, u_char tpay _U_,
17241753
size_t nmap = sizeof(oakley_t_map)/sizeof(oakley_t_map[0]);
17251754
ND_PRINT((ndo," attrs=("));
17261755
while (cp < ep && cp < ep2) {
1727-
cp = ikev1_attrmap_print(ndo, cp,
1728-
(ep < ep2) ? ep : ep2, map, nmap);
1756+
cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap);
1757+
if (cp == NULL) {
1758+
ND_PRINT((ndo,")"));
1759+
goto trunc;
1760+
}
17291761
}
17301762
ND_PRINT((ndo,")"));
17311763
break;
@@ -1926,10 +1958,11 @@ ikev2_t_print(netdissect_options *ndo, int tcount,
19261958
ep2 = (const u_char *)p + item_len;
19271959
while (cp < ep && cp < ep2) {
19281960
if (map && nmap) {
1929-
cp = ikev1_attrmap_print(ndo, cp, (ep < ep2) ? ep : ep2,
1930-
map, nmap);
1961+
cp = ikev1_attrmap_print(ndo, cp, ep2, map, nmap);
19311962
} else
1932-
cp = ikev1_attr_print(ndo, cp, (ep < ep2) ? ep : ep2);
1963+
cp = ikev1_attr_print(ndo, cp, ep2);
1964+
if (cp == NULL)
1965+
goto trunc;
19331966
}
19341967
if (ep < ep2)
19351968
ND_PRINT((ndo,"..."));

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,7 @@ ip6_frag_asan ip6_frag_asan.pcap ip6_frag_asan.out -v
553553
radius_attr_asan radius_attr_asan.pcap radius_attr_asan.out -v
554554
ospf6_decode_v3_asan ospf6_decode_v3_asan.pcap ospf6_decode_v3_asan.out -v
555555
ip_ts_opts_asan ip_ts_opts_asan.pcap ip_ts_opts_asan.out -v
556+
isakmpv1-attr-oobr isakmpv1-attr-oobr.pcap isakmpv1-attr-oobr.out -v
556557

557558
# bad packets from Katie Holly
558559
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out

Diff for: tests/isakmpv1-attr-oobr.out

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
IP (tos 0x60, ttl 254, id 40192, offset 0, flags [+, DF, rsvd], proto UDP (17), length 63264, options (unknown 255 [bad length 18]), bad cksum 8e30 (->f45)!)
2+
251.73.77.150.32514 > 126.172.128.5.500: isakmp 1.0 msgid 2200af01: phase 2/others ? #40:
3+
(t: #243 id=241 (type=#9472 len=2 value=0619) [|t]) (len mismatch: isakmp 4293885728/ip 2140)

Diff for: tests/isakmpv1-attr-oobr.pcap

135 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)