diff options
| author | Donatas Abraitis <donatas@opensourcerouting.org> | 2024-03-27 19:08:38 +0200 |
|---|---|---|
| committer | Donatas Abraitis <donatas@opensourcerouting.org> | 2024-03-29 09:13:11 +0200 |
| commit | babb23b74855e23c987a63f8256d24e28c044d07 (patch) | |
| tree | 8a32d0bf398c243b4b7127077469225ddc734571 /bgpd/bgp_attr.c | |
| parent | ba6a8f1a31e1a88df2de69ea46068e8bd9b97138 (diff) | |
bgpd: Prevent from one more CVE triggering this place
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>
Diffstat (limited to 'bgpd/bgp_attr.c')
| -rw-r--r-- | bgpd/bgp_attr.c | 33 |
1 files changed, 22 insertions, 11 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 6c886b0061..e2321ad296 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -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 @@ -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. |