CVE-2017-13053: Out-of-bounds Read (CWE-125)
value: The BGP parser in tcpdump before 4.9.2 has a buffer over-read in print-bgp.c:decode_rt_routing_info().
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-bgp.c, tests/TESTLIST, tests/bgp_vpn_rt-oobr.out
Commit date: 2017-09-13 12:25:44 +0100
Commit message: CVE-2017-13053/BGP: fix VPN route target bounds checks

decode_rt_routing_info() didn't check bounds before fetching 4 octets of
the origin AS field and could over-read the input buffer, put it right.

It also fetched the varying number of octets of the route target field
from 4 octets lower than the correct offset, put it right.

It also used the same temporary buffer explicitly through as_printf()
and implicitly through bgp_vpn_rd_print() so the end result of snprintf()
was not what was originally intended.

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-bgp.c b/print-bgp.c
index ced54c3dd2..6114540556 100644
--- a/print-bgp.c
+++ b/print-bgp.c
@@ -759,36 +759,52 @@ static int
 decode_rt_routing_info(netdissect_options *ndo,
                        const u_char *pptr, char *buf, u_int buflen)
 {
 	uint8_t route_target[8];
 	u_int plen;
+	char asbuf[sizeof(astostr)]; /* bgp_vpn_rd_print() overwrites astostr */
 
+	/* NLRI "prefix length" from RFC 2858 Section 4. */
 	ND_TCHECK(pptr[0]);
 	plen = pptr[0];   /* get prefix length */
 
+	/* NLRI "prefix" (ibid), valid lengths are { 0, 32, 33, ..., 96 } bits.
+	 * RFC 4684 Section 4 defines the layout of "origin AS" and "route
+	 * target" fields inside the "prefix" depending on its length.
+	 */
 	if (0 == plen) {
+		/* Without "origin AS", without "route target". */
 		snprintf(buf, buflen, "default route target");
 		return 1;
 	}
 
 	if (32 > plen)
 		return -1;
 
+	/* With at least "origin AS", possibly with "route target". */
+	ND_TCHECK_32BITS(pptr + 1);
+	as_printf(ndo, asbuf, sizeof(asbuf), EXTRACT_32BITS(pptr + 1));
+
         plen-=32; /* adjust prefix length */
 
 	if (64 < plen)
 		return -1;
 
+	/* From now on (plen + 7) / 8 evaluates to { 0, 1, 2, ..., 8 }
+	 * and gives the number of octets in the variable-length "route
+	 * target" field inside this NLRI "prefix". Look for it.
+	 */
 	memset(&route_target, 0, sizeof(route_target));
-	ND_TCHECK2(pptr[1], (plen + 7) / 8);
-	memcpy(&route_target, &pptr[1], (plen + 7) / 8);
+	ND_TCHECK2(pptr[5], (plen + 7) / 8);
+	memcpy(&route_target, &pptr[5], (plen + 7) / 8);
+	/* Which specification says to do this? */
 	if (plen % 8) {
 		((u_char *)&route_target)[(plen + 7) / 8 - 1] &=
 			((0xff00 >> (plen % 8)) & 0xff);
 	}
 	snprintf(buf, buflen, "origin AS: %s, route target %s",
-	    as_printf(ndo, astostr, sizeof(astostr), EXTRACT_32BITS(pptr+1)),
+	    asbuf,
 	    bgp_vpn_rd_print(ndo, (u_char *)&route_target));
 
 	return 5 + (plen + 7) / 8;
 
 trunc:
diff --git a/tests/TESTLIST b/tests/TESTLIST
index eb0839e0c0..d770f5ccf0 100644
--- a/tests/TESTLIST
+++ b/tests/TESTLIST
@@ -570,10 +570,11 @@ esis_opt_prot-oobr	esis_opt_prot-oobr.pcap		esis_opt_prot-oobr.out	-v -c1
 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
 
 # bad packets from Katie Holly
 mlppp-oobr		mlppp-oobr.pcap			mlppp-oobr.out
 
 # bad packets from Kim Gwan Yeong
diff --git a/tests/bgp_vpn_rt-oobr.out b/tests/bgp_vpn_rt-oobr.out
new file mode 100644
index 0000000000..b415962679
--- /dev/null
+++ b/tests/bgp_vpn_rt-oobr.out
@@ -0,0 +1,43 @@
+IP (tos 0xc, ttl 254, id 21263, offset 0, flags [rsvd], proto TCP (6), length 60165, bad cksum 8e15 (->9eb8)!)
+    241.0.128.19.179 > 239.8.0.1.0: Flags [none], seq 2146695561:2146755682, win 56026, options [unknown-161,eol], length 60121: BGP
+	Update Message (2), length: 45
+	  Withdrawn routes: 3 bytes
+	  Attribute Set (128), length: 7, Flags [OTPE+f]: 
+	    Origin AS: 0
+	      Multi-Protocol Unreach NLRI (15), length: 227, Flags [T+6]: 
+	    AFI: IPv6 (2), SAFI: Multicast VPN (5)
+	      Route-Type: Source-Active (5), length: 5, RD: unknown RD format, Group bogus address length 127
+	      Route-Type: Unknown (142), length: 142
+	      Route-Type: Unknown (0), length: 0
+	      Route-Type: Unknown (33), length: 0
+	      Route-Type: Unknown (0), length: 0[|BGP] [|BGP]
+	Update Message (2), length: 45[|BGP] [|BGP]
+	Update Message (2), length: 45
+	  Withdrawn routes: 3 bytes
+	  Attribute Set (128), length: 7, Flags [OTPE+f]: 
+	    Origin AS: 0
+	      Multi-Protocol Reach NLRI (14), length: 227, Flags [T+6]: 
+	    AFI: IPv4 (1), vendor specific SAFI: Route Target Routing Information (132)
+	    nexthop: invalid len, nh-length: 1, no SNPA
+	      origin AS: 0, route target 0:0 (= 0.0.0.0)
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target
+	      default route target[|BGP]
diff --git a/tests/bgp_vpn_rt-oobr.pcap b/tests/bgp_vpn_rt-oobr.pcap
new file mode 100644
index 0000000000..69d5b96066
Binary files /dev/null and b/tests/bgp_vpn_rt-oobr.pcap differ

Vulnerable code: 	ND_TCHECK2(pptr[1], (plen + 7) / 8);
	memcpy(&route_target, &pptr[1], (plen + 7) / 8);
	    as_printf(ndo, astostr, sizeof(astostr), EXTRACT_32BITS(pptr+1)),
Fixed code: 	char asbuf[sizeof(astostr)]; /* bgp_vpn_rd_print() overwrites astostr */
	/* NLRI "prefix length" from RFC 2858 Section 4. */
	/* NLRI "prefix" (ibid), valid lengths are { 0, 32, 33, ..., 96 } bits.
	 * RFC 4684 Section 4 defines the layout of "origin AS" and "route
	 * target" fields inside the "prefix" depending on its length.
	 */
		/* Without "origin AS", without "route target". */
	/* With at least "origin AS", possibly with "route target". */
	ND_TCHECK_32BITS(pptr + 1);
	as_printf(ndo, asbuf, sizeof(asbuf), EXTRACT_32BITS(pptr + 1));

	/* From now on (plen + 7) / 8 evaluates to { 0, 1, 2, ..., 8 }
	 * and gives the number of octets in the variable-length "route
	 * target" field inside this NLRI "prefix". Look for it.
	 */
	ND_TCHECK2(pptr[5], (plen + 7) / 8);
	memcpy(&route_target, &pptr[5], (plen + 7) / 8);
	/* Which specification says to do this? */
	    asbuf,
bgp_vpn_rt-oobr	bgp_vpn_rt-oobr.pcap	bgp_vpn_rt-oobr.out	-v -c1
IP (tos 0xc, ttl 254, id 21263, offset 0, flags [rsvd], proto TCP (6), length 60165, bad cksum 8e15 (->9eb8)!)
    241.0.128.19.179 > 239.8.0.1.0: Flags [none], seq 2146695561:2146755682, win 56026, options [unknown-161,eol], length 60121: BGP
	Update Message (2), length: 45
	  Withdrawn routes: 3 bytes
	  Attribute Set (128), length: 7, Flags [OTPE+f]: 
	    Origin AS: 0
	      Multi-Protocol Unreach NLRI (15), length: 227, Flags [T+6]: 
	    AFI: IPv6 (2), SAFI: Multicast VPN (5)
	      Route-Type: Source-Active (5), length: 5, RD: unknown RD format, Group bogus address length 127
	      Route-Type: Unknown (142), length: 142
	      Route-Type: Unknown (0), length: 0
	      Route-Type: Unknown (33), length: 0
	      Route-Type: Unknown (0), length: 0[|BGP] [|BGP]
	Update Message (2), length: 45[|BGP] [|BGP]
	Update Message (2), length: 45
	  Withdrawn routes: 3 bytes
	  Attribute Set (128), length: 7, Flags [OTPE+f]: 
	    Origin AS: 0
	      Multi-Protocol Reach NLRI (14), length: 227, Flags [T+6]: 
	    AFI: IPv4 (1), vendor specific SAFI: Route Target Routing Information (132)
	    nexthop: invalid len, nh-length: 1, no SNPA
	      origin AS: 0, route target 0:0 (= 0.0.0.0)
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target
	      default route target[|BGP]