From ed284e2338111ff367087bc87f5ad9c67dd9e6cc Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 11 Jan 2022 12:24:41 +0200 Subject: [PATCH] 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 --- bgpd/bgpd.c | 17 +++++++++++------ 1 file 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 */