CVE-2017-13052: Out-of-bounds Read (CWE-125)
value: The CFM parser in tcpdump before 4.9.2 has a buffer over-read in print-cfm.c:cfm_print().
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 9.8
Published: 2017-09-14T06:29Z
Repository: https://github.com/the-tcpdump-group/tcpdump
Language: Other
Files changed: print-cfm.c, tests/TESTLIST, tests/cfm_sender_id-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: 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).

Diff: diff --git a/print-cfm.c b/print-cfm.c
index 0fe57100d8..85aebb1317 100644
--- a/print-cfm.c
+++ b/print-cfm.c
@@ -215,34 +215,49 @@ static const struct tok cfm_tlv_senderid_chassisid_values[] = {
 };
 
 
 static int
 cfm_network_addr_print(netdissect_options *ndo,
-                       register const u_char *tptr)
+                       register const u_char *tptr, const u_int length)
 {
     u_int network_addr_type;
     u_int hexdump =  FALSE;
 
     /*
      * Altough AFIs are tpically 2 octects wide,
      * 802.1ab specifies that this field width
      * is only once octet
      */
+    if (length < 1) {
+        ND_PRINT((ndo, "\n\t  Network Address Type (invalid, no data"));
+        return hexdump;
+    }
+    /* The calling function must make any due ND_TCHECK calls. */
     network_addr_type = *tptr;
     ND_PRINT((ndo, "\n\t  Network Address Type %s (%u)",
            tok2str(af_values, "Unknown", network_addr_type),
            network_addr_type));
 
     /*
      * Resolve the passed in Address.
      */
     switch(network_addr_type) {
     case AFNUM_INET:
+        if (length != 1 + 4) {
+            ND_PRINT((ndo, "(invalid IPv4 address length %u)", length - 1));
+            hexdump = TRUE;
+            break;
+        }
         ND_PRINT((ndo, ", %s", ipaddr_string(ndo, tptr + 1)));
         break;
 
     case AFNUM_INET6:
+        if (length != 1 + 16) {
+            ND_PRINT((ndo, "(invalid IPv6 address length %u)", length - 1));
+            hexdump = TRUE;
+            break;
+        }
         ND_PRINT((ndo, ", %s", ip6addr_string(ndo, tptr + 1)));
         break;
 
     default:
         hexdump = TRUE;
@@ -582,25 +597,31 @@ cfm_print(netdissect_options *ndo,
             u_int chassis_id_type, chassis_id_length;
             u_int mgmt_addr_length;
 
             if (cfm_tlv_len < 1) {
                 ND_PRINT((ndo, " (too short, must be >= 1)"));
-                return;
+                goto next_tlv;
             }
 
             /*
              * Get the Chassis ID length and check it.
+             * IEEE 802.1Q-2014 Section 21.5.3.1
              */
             chassis_id_length = *tptr;
             tptr++;
             tlen--;
             cfm_tlv_len--;
 
             if (chassis_id_length) {
+                /*
+                 * IEEE 802.1Q-2014 Section 21.5.3.2: Chassis ID Subtype, references
+                 * IEEE 802.1AB-2005 Section 9.5.2.2, subsequently
+                 * IEEE 802.1AB-2016 Section 8.5.2.2: chassis ID subtype
+                 */
                 if (cfm_tlv_len < 1) {
                     ND_PRINT((ndo, "\n\t  (TLV too short)"));
-                    return;
+                    goto next_tlv;
                 }
                 chassis_id_type = *tptr;
                 cfm_tlv_len--;
                 ND_PRINT((ndo, "\n\t  Chassis-ID Type %s (%u), Chassis-ID length %u",
                        tok2str(cfm_tlv_senderid_chassisid_values,
@@ -609,20 +630,26 @@ cfm_print(netdissect_options *ndo,
                        chassis_id_type,
                        chassis_id_length));
 
                 if (cfm_tlv_len < chassis_id_length) {
                     ND_PRINT((ndo, "\n\t  (TLV too short)"));
-                    return;
+                    goto next_tlv;
                 }
 
+                /* IEEE 802.1Q-2014 Section 21.5.3.3: Chassis ID */
                 switch (chassis_id_type) {
                 case CFM_CHASSIS_ID_MAC_ADDRESS:
+                    if (chassis_id_length != ETHER_ADDR_LEN) {
+                        ND_PRINT((ndo, " (invalid MAC address length)"));
+                        hexdump = TRUE;
+                        break;
+                    }
                     ND_PRINT((ndo, "\n\t  MAC %s", etheraddr_string(ndo, tptr + 1)));
                     break;
 
                 case CFM_CHASSIS_ID_NETWORK_ADDRESS:
-                    hexdump |= cfm_network_addr_print(ndo, tptr);
+                    hexdump |= cfm_network_addr_print(ndo, tptr + 1, chassis_id_length);
                     break;
 
                 case CFM_CHASSIS_ID_INTERFACE_NAME: /* fall through */
                 case CFM_CHASSIS_ID_INTERFACE_ALIAS:
                 case CFM_CHASSIS_ID_LOCAL:
@@ -641,50 +668,66 @@ cfm_print(netdissect_options *ndo,
                 tlen -= 1 + chassis_id_length;
             }
 
             /*
              * Check if there is a Management Address.
+             * IEEE 802.1Q-2014 Section 21.5.3.4: Management Address Domain Length
+             * This and all subsequent fields are not present if the TLV length
+             * allows only the above fields.
              */
             if (cfm_tlv_len == 0) {
                 /* No, there isn't; we're done. */
-                return;
+                break;
             }
 
+            /* Here mgmt_addr_length stands for the management domain length. */
             mgmt_addr_length = *tptr;
             tptr++;
             tlen--;
             cfm_tlv_len--;
+            ND_PRINT((ndo, "\n\t  Management Address Domain Length %u", mgmt_addr_length));
             if (mgmt_addr_length) {
+                /* IEEE 802.1Q-2014 Section 21.5.3.5: Management Address Domain */
                 if (cfm_tlv_len < mgmt_addr_length) {
                     ND_PRINT((ndo, "\n\t  (TLV too short)"));
-                    return;
+                    goto next_tlv;
                 }
                 cfm_tlv_len -= mgmt_addr_length;
                 /*
                  * XXX - this is an OID; print it as such.
                  */
+                hex_print(ndo, "\n\t  Management Address Domain: ", tptr, mgmt_addr_length);
                 tptr += mgmt_addr_length;
                 tlen -= mgmt_addr_length;
 
+                /*
+                 * IEEE 802.1Q-2014 Section 21.5.3.6: Management Address Length
+                 * This field is present if Management Address Domain Length is not 0.
+                 */
                 if (cfm_tlv_len < 1) {
-                    ND_PRINT((ndo, "\n\t  (TLV too short)"));
-                    return;
+                    ND_PRINT((ndo, " (Management Address Length is missing)"));
+                    hexdump = TRUE;
+                    break;
                 }
 
+                /* Here mgmt_addr_length stands for the management address length. */
                 mgmt_addr_length = *tptr;
                 tptr++;
                 tlen--;
                 cfm_tlv_len--;
+                ND_PRINT((ndo, "\n\t  Management Address Length %u", mgmt_addr_length));
                 if (mgmt_addr_length) {
+                    /* IEEE 802.1Q-2014 Section 21.5.3.7: Management Address */
                     if (cfm_tlv_len < mgmt_addr_length) {
                         ND_PRINT((ndo, "\n\t  (TLV too short)"));
                         return;
                     }
                     cfm_tlv_len -= mgmt_addr_length;
                     /*
                      * XXX - this is a TransportDomain; print it as such.
                      */
+                    hex_print(ndo, "\n\t  Management Address: ", tptr, mgmt_addr_length);
                     tptr += mgmt_addr_length;
                     tlen -= mgmt_addr_length;
                 }
             }
             break;
@@ -704,10 +747,11 @@ cfm_print(netdissect_options *ndo,
         }
         /* do we want to see an additional hexdump ? */
         if (hexdump || ndo->ndo_vflag > 1)
             print_unknown_data(ndo, tlv_ptr, "\n\t  ", cfm_tlv_len);
 
+next_tlv:
         tptr+=cfm_tlv_len;
         tlen-=cfm_tlv_len;
     }
     return;
 
diff --git a/tests/TESTLIST b/tests/TESTLIST
index d770f5ccf0..874876924f 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -571,10 +571,11 @@ rsvp_uni-oobr-1	rsvp_uni-oobr-1.pcap	rsvp_uni-oobr-1.out	-v -c1
 rsvp_uni-oobr-2	rsvp_uni-oobr-2.pcap	rsvp_uni-oobr-2.out	-v -c1
 rsvp_uni-oobr-3	rsvp_uni-oobr-3.pcap	rsvp_uni-oobr-3.out	-v -c3
 rpki-rtr-oob		rpki-rtr-oob.pcap	rpki-rtr-oob.out	-v -c1
 lldp_8023_mtu-oobr	lldp_8023_mtu-oobr.pcap	lldp_8023_mtu-oobr.out	-v -c1
 bgp_vpn_rt-oobr	bgp_vpn_rt-oobr.pcap	bgp_vpn_rt-oobr.out	-v -c1
+cfm_sender_id-oobr	cfm_sender_id-oobr.pcap	cfm_sender_id-oobr.out	-v -c1
 
 # bad packets from Katie Holly
 mlppp-oobr		mlppp-oobr.pcap			mlppp-oobr.out
 
 # bad packets from Kim Gwan Yeong
diff --git a/tests/cfm_sender_id-oobr.out b/tests/cfm_sender_id-oobr.out
new file mode 100644
index 0000000000..4d294af909
--- /dev/null
+++ b/tests/cfm_sender_id-oobr.out
@@ -0,0 +1,8 @@
+CFMv0 unknown (255), MD Level 0, length 65556
+	First TLV offset 0
+	  0x0000:  ff00 0001 0004 0104 9a00 000c fb
+	Unknown TLV (0xff), length 0
+	Sender ID TLV (0x01), length 4
+	  Chassis-ID Type MAC address (4), Chassis-ID length 1 (invalid MAC address length)
+	  Management Address Domain Length 0
+	End TLV (0x00)
diff --git a/tests/cfm_sender_id-oobr.pcap b/tests/cfm_sender_id-oobr.pcap
new file mode 100644
index 0000000000..30d9a1feac
Binary files /dev/null and b/tests/cfm_sender_id-oobr.pcap differ

Vulnerable code:                        register const u_char *tptr)
                return;
                    return;
                    return;
                    hexdump |= cfm_network_addr_print(ndo, tptr);
                return;
                    return;
                    ND_PRINT((ndo, "\n\t  (TLV too short)"));
                    return;
Fixed code:                        register const u_char *tptr, const u_int length)
    if (length < 1) {
        ND_PRINT((ndo, "\n\t  Network Address Type (invalid, no data"));
        return hexdump;
    }
    /* The calling function must make any due ND_TCHECK calls. */
        if (length != 1 + 4) {
            ND_PRINT((ndo, "(invalid IPv4 address length %u)", length - 1));
            hexdump = TRUE;
            break;
        }
        if (length != 1 + 16) {
            ND_PRINT((ndo, "(invalid IPv6 address length %u)", length - 1));
            hexdump = TRUE;
            break;
        }
                goto next_tlv;
             * IEEE 802.1Q-2014 Section 21.5.3.1
                /*
                 * IEEE 802.1Q-2014 Section 21.5.3.2: Chassis ID Subtype, references
                 * IEEE 802.1AB-2005 Section 9.5.2.2, subsequently
                 * IEEE 802.1AB-2016 Section 8.5.2.2: chassis ID subtype
                 */
                    goto next_tlv;
                    goto next_tlv;
                /* IEEE 802.1Q-2014 Section 21.5.3.3: Chassis ID */
                    if (chassis_id_length != ETHER_ADDR_LEN) {
                        ND_PRINT((ndo, " (invalid MAC address length)"));
                        hexdump = TRUE;
                        break;
                    }
                    hexdump |= cfm_network_addr_print(ndo, tptr + 1, chassis_id_length);
             * IEEE 802.1Q-2014 Section 21.5.3.4: Management Address Domain Length
             * This and all subsequent fields are not present if the TLV length
             * allows only the above fields.
                break;
            /* Here mgmt_addr_length stands for the management domain length. */
            ND_PRINT((ndo, "\n\t  Management Address Domain Length %u", mgmt_addr_length));
                /* IEEE 802.1Q-2014 Section 21.5.3.5: Management Address Domain */
                    goto next_tlv;
                hex_print(ndo, "\n\t  Management Address Domain: ", tptr, mgmt_addr_length);
                /*
                 * IEEE 802.1Q-2014 Section 21.5.3.6: Management Address Length
                 * This field is present if Management Address Domain Length is not 0.
                 */
                    ND_PRINT((ndo, " (Management Address Length is missing)"));
                    hexdump = TRUE;
                    break;
                /* Here mgmt_addr_length stands for the management address length. */
                ND_PRINT((ndo, "\n\t  Management Address Length %u", mgmt_addr_length));
                    /* IEEE 802.1Q-2014 Section 21.5.3.7: Management Address */
                    hex_print(ndo, "\n\t  Management Address: ", tptr, mgmt_addr_length);
next_tlv:
cfm_sender_id-oobr	cfm_sender_id-oobr.pcap	cfm_sender_id-oobr.out	-v -c1
CFMv0 unknown (255), MD Level 0, length 65556
	First TLV offset 0
	  0x0000:  ff00 0001 0004 0104 9a00 000c fb
	Unknown TLV (0xff), length 0
	Sender ID TLV (0x01), length 4
	  Chassis-ID Type MAC address (4), Chassis-ID length 1 (invalid MAC address length)
	  Management Address Domain Length 0
	End TLV (0x00)