Skip to content

Commit 289c672

Browse files
committed
CVE-2017-13051/RSVP: fix bounds checks for UNI
Fixup the part of rsvp_obj_print() that decodes the GENERALIZED_UNI object from RFC 3476 Section 3.1 to check the sub-objects inside that object more thoroughly. 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).
1 parent 331530a commit 289c672

8 files changed

+43
-1
lines changed

Diff for: print-rsvp.c

+18-1
Original file line numberDiff line numberDiff line change
@@ -1205,6 +1205,17 @@ rsvp_obj_print(netdissect_options *ndo,
12051205
/* read variable length subobjects */
12061206
total_subobj_len = obj_tlen;
12071207
while(total_subobj_len > 0) {
1208+
/* If RFC 3476 Section 3.1 defined that a sub-object of the
1209+
* GENERALIZED_UNI RSVP object must have the Length field as
1210+
* a multiple of 4, instead of the check below it would be
1211+
* better to test total_subobj_len only once before the loop.
1212+
* So long as it does not define it and this while loop does
1213+
* not implement such a requirement, let's accept that within
1214+
* each iteration subobj_len may happen to be a multiple of 1
1215+
* and test it and total_subobj_len respectively.
1216+
*/
1217+
if (total_subobj_len < 4)
1218+
goto invalid;
12081219
subobj_len = EXTRACT_16BITS(obj_tptr);
12091220
subobj_type = (EXTRACT_16BITS(obj_tptr+2))>>8;
12101221
af = (EXTRACT_16BITS(obj_tptr+2))&0x00FF;
@@ -1216,7 +1227,13 @@ rsvp_obj_print(netdissect_options *ndo,
12161227
tok2str(af_values, "Unknown", af), af,
12171228
subobj_len));
12181229

1219-
if(subobj_len == 0)
1230+
/* In addition to what is explained above, the same spec does not
1231+
* explicitly say that the same Length field includes the 4-octet
1232+
* sub-object header, but as long as this while loop implements it
1233+
* as it does include, let's keep the check below consistent with
1234+
* the rest of the code.
1235+
*/
1236+
if(subobj_len < 4 || subobj_len > total_subobj_len)
12201237
goto invalid;
12211238

12221239
switch(subobj_type) {

Diff for: tests/TESTLIST

+3
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,9 @@ bgp_pmsi_tunnel-oobr bgp_pmsi_tunnel-oobr.pcap bgp_pmsi_tunnel-oobr.out -v -c1
567567
bgp_mvpn_6_and_7 bgp_mvpn_6_and_7.pcap bgp_mvpn_6_and_7.out -v -c1
568568
rsvp_fast_reroute-oobr rsvp_fast_reroute-oobr.pcap rsvp_fast_reroute-oobr.out -v -c1
569569
esis_opt_prot-oobr esis_opt_prot-oobr.pcap esis_opt_prot-oobr.out -v -c1
570+
rsvp_uni-oobr-1 rsvp_uni-oobr-1.pcap rsvp_uni-oobr-1.out -v -c1
571+
rsvp_uni-oobr-2 rsvp_uni-oobr-2.pcap rsvp_uni-oobr-2.out -v -c1
572+
rsvp_uni-oobr-3 rsvp_uni-oobr-3.pcap rsvp_uni-oobr-3.out -v -c3
570573

571574
# bad packets from Katie Holly
572575
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out

Diff for: tests/rsvp_uni-oobr-1.out

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
IP (tos 0x2,ECT(0), ttl 248, id 0, offset 0, flags [none], proto RSVP (46), length 54312, bad cksum 3743 (->7e72)!)
2+
54.35.0.0 > 58.16.0.0:
3+
RSVPv1 Hello Message (20), Flags: [Refresh reduction capable], length: 65527, ttl: 15, checksum: 0x0902
4+
Generalized UNI Object (229) Flags: [ignore and forward if unknown], Class-Type: 1 (1), length: 12
5+
Subobject Type: Unknown (127), AF: HDLC (4), length: 2 (invalid)

Diff for: tests/rsvp_uni-oobr-1.pcap

112 Bytes
Binary file not shown.

Diff for: tests/rsvp_uni-oobr-2.out

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
IP (tos 0x2,ECT(0), ttl 248, id 0, offset 0, flags [none], proto RSVP (46), length 54312, bad cksum 3743 (->3051)!)
2+
54.35.78.33 > 58.16.0.0:
3+
RSVPv1 Hello Message (20), Flags: [Refresh reduction capable], length: 65527, ttl: 15, checksum: 0x0902
4+
Generalized UNI Object (229) Flags: [ignore and forward if unknown], Class-Type: 1 (1), length: 12
5+
Subobject Type: Unknown (0), AF: HDLC (4), length: 2 (invalid)

Diff for: tests/rsvp_uni-oobr-2.pcap

112 Bytes
Binary file not shown.

Diff for: tests/rsvp_uni-oobr-3.out

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
IP (tos 0x0, ttl 48, id 25615, offset 0, flags [+, DF, rsvd], proto UDP (17), length 61735, bad cksum 8ef1 (->10e1)!)
2+
1.2.3.3.1812 > 64.112.0.96.4567: wb-29!
3+
IP (tos 0x2,ECT(0), ttl 248, id 0, offset 0, flags [none], proto RSVP (46), length 54312, bad cksum 3701 (->8972)!)
4+
54.35.0.0 > 47.16.0.0:
5+
RSVPv1 Hello Message (20), Flags: [Refresh reduction capable], length: 65527, ttl: 15, checksum: 0x0902
6+
Generalized UNI Object (229) Flags: [ignore and forward if unknown], Class-Type: 1 (1), length: 12
7+
Subobject Type: Unknown (0), AF: HDLC (4), length: 1 (invalid)
8+
IP (tos 0x2,ECT(0), ttl 248, id 0, offset 0, flags [none], proto RSVP (46), length 54312, bad cksum 3701 (->7e72)!)
9+
54.35.0.0 > 58.16.0.0:
10+
RSVPv1 Hello Message (20), Flags: [Refresh reduction capable], length: 65527, ttl: 15, checksum: 0x0902
11+
Generalized UNI Object (229) Flags: [ignore and forward if unknown], Class-Type: 1 (1), length: 12
12+
Subobject Type: Unknown (225), AF: HDLC (4), length: 1 (invalid)

Diff for: tests/rsvp_uni-oobr-3.pcap

321 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)