From e9eb324a6a35d89241428fa3282c697312aa1178 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 6 Dec 2022 10:23:11 -0500 Subject: [PATCH] bgpd: Ensure stream received has enough data BGP_PREFIX_SID_SRV6_L3_SERVICE attributes must not fully trust the length value specified in the nlri. Always ensure that the amount of data we need to read can be fullfilled. Reported-by: Iggy Frankovic Signed-off-by: Donald Sharp --- bgpd/bgp_attr.c | 79 ++++++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 54 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 8e66a22..b2cda61 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2979,9 +2979,21 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, int srgb_count; uint8_t sid_type, sid_flags; + /* + * Check that we actually have at least as much data as + * specified by the length field + */ + if (STREAM_READABLE(peer->curr) < length) { + flog_err( + EC_BGP_ATTR_LEN, + "Prefix SID specifies length %hu, but only %zu bytes remain", + length, STREAM_READABLE(peer->curr)); + return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); + } + if (type == BGP_PREFIX_SID_LABEL_INDEX) { - if (STREAM_READABLE(peer->curr) < length - || length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) { + if (length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) { flog_err(EC_BGP_ATTR_LEN, "Prefix SID label index length is %hu instead of %u", length, BGP_PREFIX_SID_LABEL_INDEX_LENGTH); @@ -3003,12 +3015,8 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, /* Store label index; subsequently, we'll check on * address-family */ attr->label_index = label_index; - } - - /* Placeholder code for the IPv6 SID type */ - else if (type == BGP_PREFIX_SID_IPV6) { - if (STREAM_READABLE(peer->curr) < length - || length != BGP_PREFIX_SID_IPV6_LENGTH) { + } else if (type == BGP_PREFIX_SID_IPV6) { + if (length != BGP_PREFIX_SID_IPV6_LENGTH) { flog_err(EC_BGP_ATTR_LEN, "Prefix SID IPv6 length is %hu instead of %u", length, BGP_PREFIX_SID_IPV6_LENGTH); @@ -3022,10 +3030,7 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, stream_getw(peer->curr); stream_get(&ipv6_sid, peer->curr, 16); - } - - /* Placeholder code for the Originator SRGB type */ - else if (type == BGP_PREFIX_SID_ORIGINATOR_SRGB) { + } else if (type == BGP_PREFIX_SID_ORIGINATOR_SRGB) { /* * ietf-idr-bgp-prefix-sid-05: * Length is the total length of the value portion of the @@ -3050,19 +3055,6 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, args->total); } - /* - * Check that we actually have at least as much data as - * specified by the length field - */ - if (STREAM_READABLE(peer->curr) < length) { - flog_err(EC_BGP_ATTR_LEN, - "Prefix SID Originator SRGB specifies length %hu, but only %zu bytes remain", - length, STREAM_READABLE(peer->curr)); - return bgp_attr_malformed( - args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); - } - /* * Check that the portion of the TLV containing the sequence of * SRGBs corresponds to a multiple of the SRGB size; to get @@ -3086,12 +3078,8 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, stream_get(&srgb_base, peer->curr, 3); stream_get(&srgb_range, peer->curr, 3); } - } - - /* Placeholder code for the VPN-SID Service type */ - else if (type == BGP_PREFIX_SID_VPN_SID) { - if (STREAM_READABLE(peer->curr) < length - || length != BGP_PREFIX_SID_VPN_SID_LENGTH) { + } else if (type == BGP_PREFIX_SID_VPN_SID) { + if (length != BGP_PREFIX_SID_VPN_SID_LENGTH) { flog_err(EC_BGP_ATTR_LEN, "Prefix SID VPN SID length is %hu instead of %u", length, BGP_PREFIX_SID_VPN_SID_LENGTH); @@ -3125,39 +3113,22 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, attr->srv6_vpn->sid_flags = sid_flags; sid_copy(&attr->srv6_vpn->sid, &ipv6_sid); attr->srv6_vpn = srv6_vpn_intern(attr->srv6_vpn); - } - - /* Placeholder code for the SRv6 L3 Service type */ - else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE) { - if (STREAM_READABLE(peer->curr) < length) { + } else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE) { + if (STREAM_READABLE(peer->curr) < 1) { flog_err( EC_BGP_ATTR_LEN, - "Prefix SID SRv6 L3-Service length is %hu, but only %zu bytes remain", - length, STREAM_READABLE(peer->curr)); - return bgp_attr_malformed(args, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); + "Prefix SID SRV6 L3 Service not enough data left, it must be at least 1 byte"); + return bgp_attr_malformed( + args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); } - /* ignore reserved */ stream_getc(peer->curr); return bgp_attr_srv6_service(args); } - /* Placeholder code for Unsupported TLV */ else { - - if (STREAM_READABLE(peer->curr) < length) { - flog_err( - EC_BGP_ATTR_LEN, - "Prefix SID SRv6 length is %hu - too long, only %zu remaining in this UPDATE", - length, STREAM_READABLE(peer->curr)); - return bgp_attr_malformed( - args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); - } - if (bgp_debug_update(peer, NULL, NULL, 1)) zlog_debug( "%s attr Prefix-SID sub-type=%u is not supported, skipped", -- 2.38.1