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>

1 files changed, 39 insertions, 8 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 7741f76a75..a7f2d9084a 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -2394,14 +2394,45 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
       attr_endp =  BGP_INPUT_PNT (peer) + length;
 
       if (attr_endp > endp)
-	{
-	  zlog_warn ("%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);
-          bgp_notify_send_with_data (peer,
-                                     BGP_NOTIFY_UPDATE_ERR,
-                                     BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-                                     startp, attr_endp - startp);
-	  return BGP_ATTR_PARSE_ERROR;
-	}
+        {
+          zlog_warn("%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, ndata, ndl + lfl + 1);
+
+          return BGP_ATTR_PARSE_ERROR;
+        }
 	
         struct bgp_attr_parser_args attr_args = {
           .peer = peer,