Skip to content

Commit

Permalink
bgpd: Prevent from one more CVE triggering this place
Browse files Browse the repository at this point in the history
If we receive an attribute that is handled by bgp_attr_malformed(), use
treat-as-withdraw behavior for unknown (or missing to add - if new) attributes.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
  • Loading branch information
ton31337 committed Mar 29, 2024
1 parent ba6a8f1 commit babb23b
Showing 1 changed file with 22 additions and 11 deletions.
33 changes: 22 additions & 11 deletions bgpd/bgp_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,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
Expand Down Expand Up @@ -1426,19 +1435,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.
Expand Down

0 comments on commit babb23b

Please sign in to comment.