From 06431bfa7570f169637ebb5898f0b0cc3b010802 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 c35e45275c9b..5b06bc391375 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2927,9 +2927,21 @@ bgp_attr_psid_sub(uint8_t type, uint16_t length, uint16_t endpoint_behavior; char buf[BUFSIZ]; + /* + * 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); @@ -2951,12 +2963,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); @@ -2970,10 +2978,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 @@ -2998,19 +3003,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 @@ -3034,12 +3026,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); @@ -2601,18 +2589,13 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length, sizeof(struct bgp_attr_srv6_vpn)); attr->srv6_vpn->sid_flags = sid_flags; sid_copy(&attr->srv6_vpn->sid, &ipv6_sid); - } - - /* Placeholder code for the SRv6 L3 Service type */ - else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE) { - if (STREAM_READABLE(peer->curr) < length - || length != BGP_PREFIX_SID_SRV6_L3_SERVICE_LENGTH) { - flog_err(EC_BGP_ATTR_LEN, - "Prefix SID SRv6 L3-Service length is %hu instead of %u", - length, BGP_PREFIX_SID_SRV6_L3_SERVICE_LENGTH); - return bgp_attr_malformed(args, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); + } 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 not enough data left, it must be at least 1 byte"); + return bgp_attr_malformed( + args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); } /* Parse L3-SERVICE Sub-TLV */ @@ -2647,17 +2630,6 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length, /* 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",