From 55bc81db12c24780aceb272d721c1de0794ee45e Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 23 Oct 2017 16:43:32 -0400 Subject: [PATCH] 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 --- bgpd/bgp_attr.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 7741f76a7..a7f2d9084 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, -- 2.39.2