diff options
author | Quentin Young <qlyoung@cumulusnetworks.com> | 2017-10-23 22:43:32 +0200 |
---|---|---|
committer | Quentin Young <qlyoung@cumulusnetworks.com> | 2017-11-07 00:38:05 +0100 |
commit | dacffad46143fb57e5fa973fcbfbd0eb51ea37b2 (patch) | |
tree | 9eaabf1bf0ee69b951a3d46a43b3f080ad898565 /bgpd/bgp_attr.c | |
parent | Merge pull request #1381 from donaldsharp/iface_desc (diff) | |
download | frr-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.c | 40 |
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; } |