diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 56e77eb..797f05d 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1381,6 +1381,15 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, (args->startp - STREAM_DATA(BGP_INPUT(peer))) + args->total); + /* Partial optional attributes that are malformed should not cause + * the whole session to be reset. Instead treat it as a withdrawal + * of the routes, if possible. + */ + if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS) && + CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL) && + CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL)) + return BGP_ATTR_PARSE_WITHDRAW; + switch (args->type) { /* where an attribute is relatively inconsequential, e.g. it does not * affect route selection, and can be safely ignored, then any such @@ -1390,6 +1399,7 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, case BGP_ATTR_AS4_AGGREGATOR: case BGP_ATTR_AGGREGATOR: case BGP_ATTR_ATOMIC_AGGREGATE: + case BGP_ATTR_PREFIX_SID: return BGP_ATTR_PARSE_PROCEED; /* Core attributes, particularly ones which may influence route @@ -1417,19 +1427,21 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, BGP_NOTIFY_UPDATE_ERR, subcode, notify_datap, length); return BGP_ATTR_PARSE_ERROR; + default: + /* Unknown attributes, that are handled by this function + * should be treated as withdraw, to prevent one more CVE + * from being introduced. + * RFC 7606 says: + * The "treat-as-withdraw" approach is generally preferred + * and the "session reset" approach is discouraged. + */ + flog_err(EC_BGP_ATTR_FLAG, + "%s(%u) attribute received, while it is not known how to handle it, treating as withdraw", + lookup_msg(attr_str, args->type, NULL), args->type); + break; } - /* Partial optional attributes that are malformed should not cause - * the whole session to be reset. Instead treat it as a withdrawal - * of the routes, if possible. - */ - if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS) - && CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL) - && CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL)) - return BGP_ATTR_PARSE_WITHDRAW; - - /* default to reset */ - return BGP_ATTR_PARSE_ERROR_NOTIFYPLS; + return BGP_ATTR_PARSE_WITHDRAW; } /* Find out what is wrong with the path attribute flag bits and log the error. @@ -3144,8 +3156,6 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args) struct attr *const attr = args->attr; enum bgp_attr_parse_ret ret; - attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID); - uint8_t type; uint16_t length; size_t headersz = sizeof(type) + sizeof(length); @@ -3195,6 +3205,8 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args) } } + SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID)); + return BGP_ATTR_PARSE_PROCEED; }