]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: Handle MP_REACH_NLRI malformed packets with session reset
authorDonatas Abraitis <donatas@opensourcerouting.org>
Fri, 20 Oct 2023 14:49:18 +0000 (17:49 +0300)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Sun, 5 May 2024 16:36:15 +0000 (16:36 +0000)
Avoid crashing bgpd.

```
(gdb)
bgp_mp_reach_parse (args=<optimized out>, mp_update=0x7fffffffe140) at bgpd/bgp_attr.c:2341
2341 stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
(gdb)
stream_get (dst=0x7fffffffe1ac, s=0x7ffff0006e80, size=16) at lib/stream.c:320
320 {
(gdb)
321 STREAM_VERIFY_SANE(s);
(gdb)
323 if (STREAM_READABLE(s) < size) {
(gdb)
34   return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
(gdb)

Thread 1 "bgpd" received signal SIGSEGV, Segmentation fault.
0x00005555556e37be in route_set_aspath_prepend (rule=0x555555aac0d0, prefix=0x7fffffffe050,
    object=0x7fffffffdb00) at bgpd/bgp_routemap.c:2282
2282 if (path->attr->aspath->refcnt)
(gdb)
```

With the configuration:

```
 neighbor 127.0.0.1 remote-as external
 neighbor 127.0.0.1 passive
 neighbor 127.0.0.1 ebgp-multihop
 neighbor 127.0.0.1 disable-connected-check
 neighbor 127.0.0.1 update-source 127.0.0.2
 neighbor 127.0.0.1 timers 3 90
 neighbor 127.0.0.1 timers connect 1
 address-family ipv4 unicast
  redistribute connected
  neighbor 127.0.0.1 default-originate
  neighbor 127.0.0.1 route-map RM_IN in
 exit-address-family
!
route-map RM_IN permit 10
 set as-path prepend 200
exit
```

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit b08afc81c60607a4f736f418f2e3eb06087f1a35)

bgpd/bgp_attr.c
bgpd/bgp_attr.h
bgpd/bgp_packet.c

index 1c0803cfd8e40c3efad02ccc8e05ef5fe4904744..4e64f8f9d9db3fc6640cf7c87ef4ce74384e0d8a 100644 (file)
@@ -2232,7 +2232,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
 
                mp_update->afi = afi;
                mp_update->safi = safi;
-               return BGP_ATTR_PARSE_EOR;
+               return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_ATTR, 0);
        }
 
        mp_update->afi = afi;
@@ -3412,10 +3412,6 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr,
                        goto done;
                }
 
-               if (ret == BGP_ATTR_PARSE_EOR) {
-                       goto done;
-               }
-
                if (ret == BGP_ATTR_PARSE_ERROR) {
                        flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR,
                                  "%s: Attribute %s, parse error", peer->host,
index 4963ea64d093edc155ea260ff753edbf215734c7..23767153b2ed30fdd9b8ab2467386d2896c4cf55 100644 (file)
@@ -382,7 +382,6 @@ enum bgp_attr_parse_ret {
        /* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR
         */
        BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
-       BGP_ATTR_PARSE_EOR = -4,
 };
 
 struct bpacket_attr_vec_arr;
index ec54943f3b4bffadf682e977696de2eeaddaae11..bf326e958dbbf0826b5c82094a0c91e526b85fb5 100644 (file)
@@ -2027,8 +2027,7 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
         * Non-MP IPv4/Unicast EoR is a completely empty UPDATE
         * and MP EoR should have only an empty MP_UNREACH
         */
-       if ((!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0)
-           || (attr_parse_ret == BGP_ATTR_PARSE_EOR)) {
+       if (!update_len && !withdraw_len && nlris[NLRI_MP_UPDATE].length == 0) {
                afi_t afi = 0;
                safi_t safi;
                struct graceful_restart_info *gr_info;
@@ -2049,9 +2048,6 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size)
                           && nlris[NLRI_MP_WITHDRAW].length == 0) {
                        afi = nlris[NLRI_MP_WITHDRAW].afi;
                        safi = nlris[NLRI_MP_WITHDRAW].safi;
-               } else if (attr_parse_ret == BGP_ATTR_PARSE_EOR) {
-                       afi = nlris[NLRI_MP_UPDATE].afi;
-                       safi = nlris[NLRI_MP_UPDATE].safi;
                }
 
                if (afi && peer->afc[afi][safi]) {