diff options
| author | Donatas Abraitis <donatas.abraitis@gmail.com> | 2022-01-11 12:24:41 +0200 |
|---|---|---|
| committer | Donatas Abraitis <donatas.abraitis@gmail.com> | 2022-01-11 17:07:19 +0200 |
| commit | ed284e2338111ff367087bc87f5ad9c67dd9e6cc (patch) | |
| tree | a8773640d6b15b2cc62792d0a70d950612c97b16 | |
| parent | 0064d4736d8e0b9815a9dd6c57ad44de51aed970 (diff) | |
bgpd: Make sure we are playing with non-NULL for bgp shutdown message
```
*** 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>
| -rw-r--r-- | bgpd/bgpd.c | 17 |
1 files changed, 11 insertions, 6 deletions
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 6d0fd2fdd6..56e299e4cc 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -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 */ |