matthieu/frr
1
0
Fork 0
forked from Mirror/frr
Code Pull requests Activity

Merge pull request #5418 from qlyoung/fix-bgp-prefix-sid-missing-boundscheck

Browse source 
bgpd: fix missing bounds checks for psid attr
This commit is contained in:
Donatas Abraitis 2020-01-08 21:59:07 +02:00 committed by GitHub
parent f84f7121f6 473046ee50
commit f20b3184b8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 99 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

134
bgpd/bgp_attr.c
View file

@ -2136,8 +2136,7 @@ static int bgp_attr_encap(uint8_t type, struct peer *peer, /* IN */
* Read an individual SID value returning how much data we have read
* Returns 0 if there was an error that needs to be passed up the stack
*/
static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
int32_t length,
static bgp_attr_parse_ret_t bgp_attr_psid_sub(uint8_t type, uint16_t length,
struct bgp_attr_parser_args *args,
struct bgp_nlri *mp_update)
{
@ -2150,11 +2149,12 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
int srgb_count;
if (type == BGP_PREFIX_SID_LABEL_INDEX) {
if (length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) {
flog_err(
EC_BGP_ATTR_LEN,
"Prefix SID label index length is %d instead of %d",
length, BGP_PREFIX_SID_LABEL_INDEX_LENGTH);
if (STREAM_READABLE(peer->curr) < length
|| length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) {
flog_err(EC_BGP_ATTR_LEN,
"Prefix SID label index length is %" PRIu16
" instead of %u",
length, BGP_PREFIX_SID_LABEL_INDEX_LENGTH);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
@ -2186,9 +2186,11 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
/* Placeholder code for the IPv6 SID type */
else if (type == BGP_PREFIX_SID_IPV6) {
if (length != BGP_PREFIX_SID_IPV6_LENGTH) {
if (STREAM_READABLE(peer->curr) < length
|| length != BGP_PREFIX_SID_IPV6_LENGTH) {
flog_err(EC_BGP_ATTR_LEN,
"Prefix SID IPv6 length is %d instead of %d",
"Prefix SID IPv6 length is %" PRIu16
" instead of %u",
length, BGP_PREFIX_SID_IPV6_LENGTH);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
@ -2204,15 +2206,54 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
/* Placeholder code for the Originator SRGB type */
else if (type == BGP_PREFIX_SID_ORIGINATOR_SRGB) {
/* Ignore flags */
/*
* ietf-idr-bgp-prefix-sid-05:
* Length is the total length of the value portion of the
* TLV: 2 + multiple of 6.
*
* peer->curr stream readp should be at the beginning of the 16
* bit flag field at this point in the code.
*/
/*
* Check that the TLV length field is sane: at least 2 bytes of
* flag, and at least 1 SRGB (these are 6 bytes each)
*/
if (length < (2 + BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH)) {
flog_err(
EC_BGP_ATTR_LEN,
"Prefix SID Originator SRGB length field claims length of %" PRIu16 " bytes, but the minimum for this TLV type is %u",
length,
2 + BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH);
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
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 %" PRIu16 ", 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
* that length, we skip the 16 bit flags field
*/
stream_getw(peer->curr);
length -= 2;
if (length % BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH) {
flog_err(
EC_BGP_ATTR_LEN,
"Prefix SID Originator SRGB length is %d, it must be a multiple of %d ",
"Prefix SID Originator SRGB length field claims attribute SRGB sequence section is %" PRIu16 "bytes, but it must be a multiple of %u",
length, BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH);
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
@ -2234,12 +2275,24 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
*/
else if (type == BGP_PREFIX_SID_SRV6_L3_SERVICE
|| type == BGP_PREFIX_SID_SRV6_L2_SERVICE) {
if (STREAM_READABLE(peer->curr) < length) {
flog_err(
EC_BGP_ATTR_LEN,
"Prefix SID SRv6 length is %" PRIu16
" - 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",
peer->host, type);
for (int i = 0; i < length; i++)
stream_getc(peer->curr);
stream_forward_getp(peer->curr, length);
}
return BGP_ATTR_PARSE_PROCEED;
@ -2248,9 +2301,8 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
/* Prefix SID attribute
* draft-ietf-idr-bgp-prefix-sid-05
*/
bgp_attr_parse_ret_t
bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args,
struct bgp_nlri *mp_update)
bgp_attr_parse_ret_t bgp_attr_prefix_sid(struct bgp_attr_parser_args *args,
struct bgp_nlri *mp_update)
{
struct peer *const peer = args->peer;
struct attr *const attr = args->attr;
@ -2258,31 +2310,40 @@ bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args,
attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID);
while (tlength) {
int32_t type, length;
uint8_t type;
uint16_t length;
size_t headersz = sizeof(type) + sizeof(length);
while (STREAM_READABLE(peer->curr) > 0) {
if (STREAM_READABLE(peer->curr) < headersz) {
flog_err(
EC_BGP_ATTR_LEN,
"Malformed Prefix SID attribute - insufficent data (need %zu for attribute header, have %zu remaining in UPDATE)",
headersz, STREAM_READABLE(peer->curr));
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
type = stream_getc(peer->curr);
length = stream_getw(peer->curr);
if (STREAM_READABLE(peer->curr) < length) {
flog_err(
EC_BGP_ATTR_LEN,
"Malformed Prefix SID attribute - insufficient data (need %" PRIu8
" for attribute body, have %zu remaining in UPDATE)",
length, STREAM_READABLE(peer->curr));
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
ret = bgp_attr_psid_sub(type, length, args, mp_update);
if (ret != BGP_ATTR_PARSE_PROCEED)
return ret;
/*
* Subtract length + the T and the L
* since length is the Vector portion
*/
tlength -= length + 3;
if (tlength < 0) {
flog_err(
EC_BGP_ATTR_LEN,
"Prefix SID internal length %d causes us to read beyond the total Prefix SID length",
length);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
}
return BGP_ATTR_PARSE_PROCEED;
@ -2678,8 +2739,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
startp);
break;
case BGP_ATTR_PREFIX_SID:
ret = bgp_attr_prefix_sid(length,
&attr_args, mp_update);
ret = bgp_attr_prefix_sid(&attr_args, mp_update);
break;
case BGP_ATTR_PMSI_TUNNEL:
ret = bgp_attr_pmsi_tunnel(&attr_args);

2
bgpd/bgp_attr.h
View file

@ -319,7 +319,7 @@ extern int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
extern int bgp_mp_unreach_parse(struct bgp_attr_parser_args *args,
struct bgp_nlri *);
extern bgp_attr_parse_ret_t
bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args,
bgp_attr_prefix_sid(struct bgp_attr_parser_args *args,
struct bgp_nlri *mp_update);
extern struct bgp_attr_encap_subtlv *

2
tests/bgpd/test_mp_attr.c
View file

@ -1027,7 +1027,7 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type)
parse_ret = bgp_mp_unreach_parse(&attr_args, &nlri);
break;
case BGP_ATTR_PREFIX_SID:
parse_ret = bgp_attr_prefix_sid(t->len, &attr_args, &nlri);
parse_ret = bgp_attr_prefix_sid(&attr_args, &nlri);
break;
default:
printf("unknown type");