CVE-2017-13045: Out-of-bounds Read (CWE-125)
value: The VQP parser in tcpdump before 4.9.2 has a buffer over-read in print-vqp.c:vqp_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-vqp.c, tests/TESTLIST, tests/vqp-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13045/VQP: add some bounds checks

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-vqp.c b/print-vqp.c
index 44a2193523..90cf8ddfef 100644
--- a/print-vqp.c
+++ b/print-vqp.c
@@ -24,10 +24,11 @@
 #include <netdissect-stdinc.h>
 
 #include "netdissect.h"
 #include "extract.h"
 #include "addrtoname.h"
+#include "ether.h"
 
 #define VQP_VERSION            		1
 #define VQP_EXTRACT_VERSION(x) ((x)&0xFF)
 
 /*
@@ -103,17 +104,19 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
     const struct vqp_obj_tlv_t *vqp_obj_tlv;
 
     const u_char *tptr;
     uint16_t vqp_obj_len;
     uint32_t vqp_obj_type;
-    int tlen;
+    u_int tlen;
     uint8_t nitems;
 
     tptr=pptr;
     tlen = len;
     vqp_common_header = (const struct vqp_common_header_t *)pptr;
     ND_TCHECK(*vqp_common_header);
+    if (sizeof(struct vqp_common_header_t) > tlen)
+        goto trunc;
 
     /*
      * Sanity checking of the header.
      */
     if (VQP_EXTRACT_VERSION(vqp_common_header->version) != VQP_VERSION) {
@@ -149,10 +152,13 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
     tlen-=sizeof(const struct vqp_common_header_t);
 
     while (nitems > 0 && tlen > 0) {
 
         vqp_obj_tlv = (const struct vqp_obj_tlv_t *)tptr;
+        ND_TCHECK(*vqp_obj_tlv);
+        if (sizeof(struct vqp_obj_tlv_t) > tlen)
+            goto trunc;
         vqp_obj_type = EXTRACT_32BITS(vqp_obj_tlv->obj_type);
         vqp_obj_len = EXTRACT_16BITS(vqp_obj_tlv->obj_length);
         tptr+=sizeof(struct vqp_obj_tlv_t);
         tlen-=sizeof(struct vqp_obj_tlv_t);
 
@@ -165,13 +171,17 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
             return;
         }
 
         /* did we capture enough for fully decoding the object ? */
         ND_TCHECK2(*tptr, vqp_obj_len);
+        if (vqp_obj_len > tlen)
+            goto trunc;
 
         switch(vqp_obj_type) {
 	case VQP_OBJ_IP_ADDRESS:
+            if (vqp_obj_len != 4)
+                goto trunc;
             ND_PRINT((ndo, "%s (0x%08x)", ipaddr_string(ndo, tptr), EXTRACT_32BITS(tptr)));
             break;
             /* those objects have similar semantics - fall through */
         case VQP_OBJ_PORT_NAME:
 	case VQP_OBJ_VLAN_NAME:
@@ -180,10 +190,12 @@ vqp_print(netdissect_options *ndo, register const u_char *pptr, register u_int l
             safeputs(ndo, tptr, vqp_obj_len);
             break;
             /* those objects have similar semantics - fall through */
 	case VQP_OBJ_MAC_ADDRESS:
 	case VQP_OBJ_MAC_NULL:
+            if (vqp_obj_len != ETHER_ADDR_LEN)
+                goto trunc;
 	      ND_PRINT((ndo, "%s", etheraddr_string(ndo, tptr)));
               break;
         default:
             if (ndo->ndo_vflag <= 1)
                 print_unknown_data(ndo,tptr, "\n\t    ", vqp_obj_len);
diff --git a/tests/TESTLIST b/tests/TESTLIST
index b42ba5b149..687e92f378 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -560,10 +560,11 @@ isakmpv1-attr-oobr	isakmpv1-attr-oobr.pcap		isakmpv1-attr-oobr.out	-v
 # code path and will not test the vulnerability unless modified respectively.
 # The .pcap file is truncated after the 1st packet.
 hncp_dhcpv6data-oobr	hncp_dhcpv6data-oobr.pcap	hncp_dhcpv6data-oobr.out -v -c1
 # Same comments apply to the case below.
 hncp_dhcpv4data-oobr	hncp_dhcpv4data-oobr.pcap	hncp_dhcpv4data-oobr.out -v -c1
+vqp-oobr		vqp-oobr.pcap			vqp-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/vqp-oobr.out b/tests/vqp-oobr.out
new file mode 100644
index 0000000000..3b1f71e366
--- /dev/null
+++ b/tests/vqp-oobr.out
@@ -0,0 +1,4 @@
+IP (tos 0x0, ttl 17, id 40207, offset 0, flags [+, DF, rsvd], proto UDP (17), length 46, bad cksum 8f04 (->f897)!)
+    0.0.128.20.1589 > 12.251.167.8.62720: 
+	VQPv1, unknown (127) Message, error-code unknown (31) (31), seq 0x80f90000, items 27, length 18
+	[|VQP]
diff --git a/tests/vqp-oobr.pcap b/tests/vqp-oobr.pcap
new file mode 100644
index 0000000000..50be013da7
Binary files /dev/null and b/tests/vqp-oobr.pcap differ

Vulnerable code:     int tlen;
Fixed code: #include "ether.h"
    u_int tlen;
    if (sizeof(struct vqp_common_header_t) > tlen)
        goto trunc;
        ND_TCHECK(*vqp_obj_tlv);
        if (sizeof(struct vqp_obj_tlv_t) > tlen)
            goto trunc;
        if (vqp_obj_len > tlen)
            goto trunc;
            if (vqp_obj_len != 4)
                goto trunc;
            if (vqp_obj_len != ETHER_ADDR_LEN)
                goto trunc;
vqp-oobr		vqp-oobr.pcap			vqp-oobr.out		-v -c1
IP (tos 0x0, ttl 17, id 40207, offset 0, flags [+, DF, rsvd], proto UDP (17), length 46, bad cksum 8f04 (->f897)!)
    0.0.128.20.1589 > 12.251.167.8.62720: 
	VQPv1, unknown (127) Message, error-code unknown (31) (31), seq 0x80f90000, items 27, length 18
	[|VQP]