Skip to content

Commit 5d340a5

Browse files
committed
CVE-2017-13052/CFM: refine decoding of the Sender ID TLV
In cfm_network_addr_print() add a length argument and use it to validate the input buffer. In cfm_print() add a length check for MAC address chassis ID. Supply cfm_network_addr_print() with the length of its buffer and a correct pointer to the buffer (it was off-by-one before). Change some error handling blocks to skip to the next TLV in the current PDU rather than to stop decoding the PDU. Print the management domain and address contents, although in hex only so far. Add some comments to clarify the code flow and to tell exact sections in IEEE standard documents. Add new error messages and make some existing messages more specific. 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 bd4e697 commit 5d340a5

File tree

4 files changed

+62
-9
lines changed

4 files changed

+62
-9
lines changed

Diff for: print-cfm.c

+53-9
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ static const struct tok cfm_tlv_senderid_chassisid_values[] = {
217217

218218
static int
219219
cfm_network_addr_print(netdissect_options *ndo,
220-
register const u_char *tptr)
220+
register const u_char *tptr, const u_int length)
221221
{
222222
u_int network_addr_type;
223223
u_int hexdump = FALSE;
@@ -227,6 +227,11 @@ cfm_network_addr_print(netdissect_options *ndo,
227227
* 802.1ab specifies that this field width
228228
* is only once octet
229229
*/
230+
if (length < 1) {
231+
ND_PRINT((ndo, "\n\t Network Address Type (invalid, no data"));
232+
return hexdump;
233+
}
234+
/* The calling function must make any due ND_TCHECK calls. */
230235
network_addr_type = *tptr;
231236
ND_PRINT((ndo, "\n\t Network Address Type %s (%u)",
232237
tok2str(af_values, "Unknown", network_addr_type),
@@ -237,10 +242,20 @@ cfm_network_addr_print(netdissect_options *ndo,
237242
*/
238243
switch(network_addr_type) {
239244
case AFNUM_INET:
245+
if (length != 1 + 4) {
246+
ND_PRINT((ndo, "(invalid IPv4 address length %u)", length - 1));
247+
hexdump = TRUE;
248+
break;
249+
}
240250
ND_PRINT((ndo, ", %s", ipaddr_string(ndo, tptr + 1)));
241251
break;
242252

243253
case AFNUM_INET6:
254+
if (length != 1 + 16) {
255+
ND_PRINT((ndo, "(invalid IPv6 address length %u)", length - 1));
256+
hexdump = TRUE;
257+
break;
258+
}
244259
ND_PRINT((ndo, ", %s", ip6addr_string(ndo, tptr + 1)));
245260
break;
246261

@@ -584,21 +599,27 @@ cfm_print(netdissect_options *ndo,
584599

585600
if (cfm_tlv_len < 1) {
586601
ND_PRINT((ndo, " (too short, must be >= 1)"));
587-
return;
602+
goto next_tlv;
588603
}
589604

590605
/*
591606
* Get the Chassis ID length and check it.
607+
* IEEE 802.1Q-2014 Section 21.5.3.1
592608
*/
593609
chassis_id_length = *tptr;
594610
tptr++;
595611
tlen--;
596612
cfm_tlv_len--;
597613

598614
if (chassis_id_length) {
615+
/*
616+
* IEEE 802.1Q-2014 Section 21.5.3.2: Chassis ID Subtype, references
617+
* IEEE 802.1AB-2005 Section 9.5.2.2, subsequently
618+
* IEEE 802.1AB-2016 Section 8.5.2.2: chassis ID subtype
619+
*/
599620
if (cfm_tlv_len < 1) {
600621
ND_PRINT((ndo, "\n\t (TLV too short)"));
601-
return;
622+
goto next_tlv;
602623
}
603624
chassis_id_type = *tptr;
604625
cfm_tlv_len--;
@@ -611,16 +632,22 @@ cfm_print(netdissect_options *ndo,
611632

612633
if (cfm_tlv_len < chassis_id_length) {
613634
ND_PRINT((ndo, "\n\t (TLV too short)"));
614-
return;
635+
goto next_tlv;
615636
}
616637

638+
/* IEEE 802.1Q-2014 Section 21.5.3.3: Chassis ID */
617639
switch (chassis_id_type) {
618640
case CFM_CHASSIS_ID_MAC_ADDRESS:
641+
if (chassis_id_length != ETHER_ADDR_LEN) {
642+
ND_PRINT((ndo, " (invalid MAC address length)"));
643+
hexdump = TRUE;
644+
break;
645+
}
619646
ND_PRINT((ndo, "\n\t MAC %s", etheraddr_string(ndo, tptr + 1)));
620647
break;
621648

622649
case CFM_CHASSIS_ID_NETWORK_ADDRESS:
623-
hexdump |= cfm_network_addr_print(ndo, tptr);
650+
hexdump |= cfm_network_addr_print(ndo, tptr + 1, chassis_id_length);
624651
break;
625652

626653
case CFM_CHASSIS_ID_INTERFACE_NAME: /* fall through */
@@ -643,38 +670,53 @@ cfm_print(netdissect_options *ndo,
643670

644671
/*
645672
* Check if there is a Management Address.
673+
* IEEE 802.1Q-2014 Section 21.5.3.4: Management Address Domain Length
674+
* This and all subsequent fields are not present if the TLV length
675+
* allows only the above fields.
646676
*/
647677
if (cfm_tlv_len == 0) {
648678
/* No, there isn't; we're done. */
649-
return;
679+
break;
650680
}
651681

682+
/* Here mgmt_addr_length stands for the management domain length. */
652683
mgmt_addr_length = *tptr;
653684
tptr++;
654685
tlen--;
655686
cfm_tlv_len--;
687+
ND_PRINT((ndo, "\n\t Management Address Domain Length %u", mgmt_addr_length));
656688
if (mgmt_addr_length) {
689+
/* IEEE 802.1Q-2014 Section 21.5.3.5: Management Address Domain */
657690
if (cfm_tlv_len < mgmt_addr_length) {
658691
ND_PRINT((ndo, "\n\t (TLV too short)"));
659-
return;
692+
goto next_tlv;
660693
}
661694
cfm_tlv_len -= mgmt_addr_length;
662695
/*
663696
* XXX - this is an OID; print it as such.
664697
*/
698+
hex_print(ndo, "\n\t Management Address Domain: ", tptr, mgmt_addr_length);
665699
tptr += mgmt_addr_length;
666700
tlen -= mgmt_addr_length;
667701

702+
/*
703+
* IEEE 802.1Q-2014 Section 21.5.3.6: Management Address Length
704+
* This field is present if Management Address Domain Length is not 0.
705+
*/
668706
if (cfm_tlv_len < 1) {
669-
ND_PRINT((ndo, "\n\t (TLV too short)"));
670-
return;
707+
ND_PRINT((ndo, " (Management Address Length is missing)"));
708+
hexdump = TRUE;
709+
break;
671710
}
672711

712+
/* Here mgmt_addr_length stands for the management address length. */
673713
mgmt_addr_length = *tptr;
674714
tptr++;
675715
tlen--;
676716
cfm_tlv_len--;
717+
ND_PRINT((ndo, "\n\t Management Address Length %u", mgmt_addr_length));
677718
if (mgmt_addr_length) {
719+
/* IEEE 802.1Q-2014 Section 21.5.3.7: Management Address */
678720
if (cfm_tlv_len < mgmt_addr_length) {
679721
ND_PRINT((ndo, "\n\t (TLV too short)"));
680722
return;
@@ -683,6 +725,7 @@ cfm_print(netdissect_options *ndo,
683725
/*
684726
* XXX - this is a TransportDomain; print it as such.
685727
*/
728+
hex_print(ndo, "\n\t Management Address: ", tptr, mgmt_addr_length);
686729
tptr += mgmt_addr_length;
687730
tlen -= mgmt_addr_length;
688731
}
@@ -706,6 +749,7 @@ cfm_print(netdissect_options *ndo,
706749
if (hexdump || ndo->ndo_vflag > 1)
707750
print_unknown_data(ndo, tlv_ptr, "\n\t ", cfm_tlv_len);
708751

752+
next_tlv:
709753
tptr+=cfm_tlv_len;
710754
tlen-=cfm_tlv_len;
711755
}

Diff for: tests/TESTLIST

+1
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,7 @@ rsvp_uni-oobr-3 rsvp_uni-oobr-3.pcap rsvp_uni-oobr-3.out -v -c3
573573
rpki-rtr-oob rpki-rtr-oob.pcap rpki-rtr-oob.out -v -c1
574574
lldp_8023_mtu-oobr lldp_8023_mtu-oobr.pcap lldp_8023_mtu-oobr.out -v -c1
575575
bgp_vpn_rt-oobr bgp_vpn_rt-oobr.pcap bgp_vpn_rt-oobr.out -v -c1
576+
cfm_sender_id-oobr cfm_sender_id-oobr.pcap cfm_sender_id-oobr.out -v -c1
576577

577578
# bad packets from Katie Holly
578579
mlppp-oobr mlppp-oobr.pcap mlppp-oobr.out

Diff for: tests/cfm_sender_id-oobr.out

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CFMv0 unknown (255), MD Level 0, length 65556
2+
First TLV offset 0
3+
0x0000: ff00 0001 0004 0104 9a00 000c fb
4+
Unknown TLV (0xff), length 0
5+
Sender ID TLV (0x01), length 4
6+
Chassis-ID Type MAC address (4), Chassis-ID length 1 (invalid MAC address length)
7+
Management Address Domain Length 0
8+
End TLV (0x00)

Diff for: tests/cfm_sender_id-oobr.pcap

94 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)