summaryrefslogtreecommitdiffstats
path: root/bgpd/bgp_attr.c
diff options
context:
space:
mode:
authorQuentin Young <qlyoung@cumulusnetworks.com>2017-10-23 22:43:32 +0200
committerQuentin Young <qlyoung@cumulusnetworks.com>2017-11-07 00:38:05 +0100
commitdacffad46143fb57e5fa973fcbfbd0eb51ea37b2 (patch)
tree9eaabf1bf0ee69b951a3d46a43b3f080ad898565 /bgpd/bgp_attr.c
parentMerge pull request #1381 from donaldsharp/iface_desc (diff)
downloadfrr-dacffad46143fb57e5fa973fcbfbd0eb51ea37b2.tar.xz
frr-dacffad46143fb57e5fa973fcbfbd0eb51ea37b2.zip
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>
Diffstat (limited to 'bgpd/bgp_attr.c')
-rw-r--r--bgpd/bgp_attr.c40
1 files changed, 38 insertions, 2 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 97cfeee4e..01a6a567b 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -2281,10 +2281,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;
}