]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: Make sure we are playing with non-NULL for bgp shutdown message 10317/head
authorDonatas Abraitis <donatas.abraitis@gmail.com>
Tue, 11 Jan 2022 10:24:41 +0000 (12:24 +0200)
committerDonatas Abraitis <donatas.abraitis@gmail.com>
Tue, 11 Jan 2022 15:07:19 +0000 (17:07 +0200)
```
*** CID 1510738:  Null pointer dereferences  (FORWARD_NULL)
/bgpd/bgp_vty.c: 4243 in bgp_shutdown_msg_magic()
4237            if (msgstr && strlen(msgstr) > BGP_ADMIN_SHUTDOWN_MSG_LEN) {
4238                    vty_out(vty, "%% Shutdown message size exceeded %d\n",
4239                            BGP_ADMIN_SHUTDOWN_MSG_LEN);
4240                    return CMD_WARNING_CONFIG_FAILED;
4241            }
4242
>>>     CID 1510738:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "msgstr" to "bgp_shutdown_enable", which dereferences it.
4243            bgp_shutdown_enable(bgp, msgstr);
4244            XFREE(MTYPE_TMP, msgstr);
4245
4246            return CMD_SUCCESS;
4247     }
4248
```

```
*** CID 1510737:  Null pointer dereferences  (REVERSE_INULL)
/bgpd/bgpd.c: 4344 in bgp_shutdown_enable()
4338                    /* continue, if peer is already in administrative shutdown. */
4339                    if (CHECK_FLAG(peer->flags, PEER_FLAG_SHUTDOWN))
4340                            continue;
4341
4342                    /* send a RFC 4486 notification message if necessary */
4343                    if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) {
>>>     CID 1510737:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "msg" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
4344                            if (msg)
4345                                    bgp_notify_send_with_data(
4346                                            peer, BGP_NOTIFY_CEASE,
4347                                            BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN, data,
4348                                            datalen + 1);
4349                            else
```

Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>
bgpd/bgpd.c

index 6d0fd2fdd6b2ad9e8ed2d9818d058cd84942b39b..56e299e4cca30fb1802caed73f69ee066597364d 100644 (file)
@@ -4320,10 +4320,6 @@ void bgp_shutdown_enable(struct bgp *bgp, const char *msg)
        struct listnode *node;
        /* length(1) + message(N) */
        uint8_t data[BGP_ADMIN_SHUTDOWN_MSG_LEN + 1];
-       size_t datalen = strlen(msg);
-
-       data[0] = datalen;
-       memcpy(data + 1, msg, datalen);
 
        /* do nothing if already shut down */
        if (CHECK_FLAG(bgp->flags, BGP_FLAG_SHUTDOWN))
@@ -4341,15 +4337,24 @@ void bgp_shutdown_enable(struct bgp *bgp, const char *msg)
 
                /* send a RFC 4486 notification message if necessary */
                if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) {
-                       if (msg)
+                       if (msg) {
+                               size_t datalen = strlen(msg);
+
+                               if (datalen > BGP_ADMIN_SHUTDOWN_MSG_LEN)
+                                       datalen = BGP_ADMIN_SHUTDOWN_MSG_LEN;
+
+                               data[0] = datalen;
+                               memcpy(data + 1, msg, datalen);
+
                                bgp_notify_send_with_data(
                                        peer, BGP_NOTIFY_CEASE,
                                        BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN, data,
                                        datalen + 1);
-                       else
+                       } else {
                                bgp_notify_send(
                                        peer, BGP_NOTIFY_CEASE,
                                        BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN);
+                       }
                }
 
                /* reset start timer to initial value */