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>
(cherry picked from commit babb23b74855e23c987a63f8256d24e28c044d07)

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 2639ff8640a05bb25401de9fb4af0d0e9c691eaf..797f05d6063277a86e0a864e0a55db221a9ae84a 100644 (file)
--- 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
@@ -1418,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.