From: Quentin Young <qlyoung@cumulusnetworks.com>
Date: Mon, 23 Oct 2017 20:43:32 +0000 (-0400)
Subject: bgpd: fix mishandled attribute length
X-Git-Tag: FRR-3.0.1^2
X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=49ba74611fd0e09045f4b9b247169f13aa364471;p=mirror_frr.git

bgpd: fix mishandled attribute length

A crafted BGP UPDATE with a malformed path attribute length field causes
bgpd to dump up to 65535 bytes of application memory and send it as the
data field in a BGP NOTIFY message, which is truncated to 4075 bytes
after accounting for protocol headers. After reading a malformed length
field, a NOTIFY is generated that is supposed to contain the problematic
data, but the malformed length field is inadvertently used to compute
how much data we send.

CVE-2017-15865

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
---

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index d79cdef10..3f8367598 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -2334,10 +2334,46 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
 				"%s: BGP type %d length %d is too large, attribute total length is %d.  attr_endp is %p.  endp is %p",
 				peer->host, type, length, size, attr_endp,
 				endp);
+			/*
+			 * RFC 4271 6.3
+			 * If any recognized attribute has an Attribute
+			 * Length that conflicts with the expected length
+			 * (based on the attribute type code), then the
+			 * Error Subcode MUST be set to Attribute Length
+			 * Error.  The Data field MUST contain the erroneous
+			 * attribute (type, length, and value).
+			 * ----------
+			 * We do not currently have a good way to determine the
+			 * length of the attribute independent of the length
+			 * received in the message. Instead we send the
+			 * minimum between the amount of data we have and the
+			 * amount specified by the attribute length field.
+			 *
+			 * Instead of directly passing in the packet buffer and
+			 * offset we use the stream_get* functions to read into
+			 * a stack buffer, since they perform bounds checking
+			 * and we are working with untrusted data.
+			 */
+			unsigned char ndata[BGP_MAX_PACKET_SIZE];
+			memset(ndata, 0x00, sizeof(ndata));
+			size_t lfl =
+				CHECK_FLAG(flag, BGP_ATTR_FLAG_EXTLEN) ? 2 : 1;
+			/* Rewind to end of flag field */
+			stream_forward_getp(BGP_INPUT(peer), -(1 + lfl));
+			/* Type */
+			stream_get(&ndata[0], BGP_INPUT(peer), 1);
+			/* Length */
+			stream_get(&ndata[1], BGP_INPUT(peer), lfl);
+			/* Value */
+			size_t atl = attr_endp - startp;
+			size_t ndl = MIN(atl, STREAM_READABLE(BGP_INPUT(peer)));
+			stream_get(&ndata[lfl + 1], BGP_INPUT(peer), ndl);
+
 			bgp_notify_send_with_data(
 				peer, BGP_NOTIFY_UPDATE_ERR,
-				BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, startp,
-				attr_endp - startp);
+				BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata,
+				ndl + lfl + 1);
+
 			return BGP_ATTR_PARSE_ERROR;
 		}