From 406f99f81d8d77bc5b2905afec2c4ed53739b306 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Fri, 2 Jun 2017 20:53:37 +0000 Subject: [PATCH] bgpd: fix addpath buffer overrun Signed-off-by: Quentin Young --- bgpd/bgp_debug.c | 11 +++++++---- bgpd/bgp_updgrp_packet.c | 35 +++++++++++++++++++++-------------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c index 232f53c778..fb350d51a1 100644 --- a/bgpd/bgp_debug.c +++ b/bgpd/bgp_debug.c @@ -40,8 +40,6 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #include "bgpd/bgp_updgrp.h" #include "bgpd/bgp_mplsvpn.h" -#define BGP_ADDPATH_STR 20 - unsigned long conf_bgp_debug_as4; unsigned long conf_bgp_debug_neighbor_events; unsigned long conf_bgp_debug_events; @@ -2132,7 +2130,12 @@ bgp_debug_rdpfxpath2str (struct prefix_rd *prd, union prefixconstptr pu, { char rd_buf[RD_ADDRSTRLEN]; char pfx_buf[PREFIX_STRLEN]; - char pathid_buf[BGP_ADDPATH_STR]; + /* ' with addpath ID ' 17 + * max strlen of uint32 + 10 + * +/- (in case of idiocy) + 1 + * null terminator + 1 + * ============================ 29 */ + char pathid_buf[30]; if (size < BGP_PRD_PATH_STRLEN) return NULL; @@ -2140,7 +2143,7 @@ bgp_debug_rdpfxpath2str (struct prefix_rd *prd, union prefixconstptr pu, /* Note: Path-id is created by default, but only included in update sometimes. */ pathid_buf[0] = '\0'; if (addpath_valid) - sprintf(pathid_buf, " with addpath ID %d", addpath_id); + snprintf(pathid_buf, sizeof(pathid_buf), " with addpath ID %u", addpath_id); if (prd) snprintf (str, size, "RD %s %s%s", diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index 5872ca6f3f..588f8a66fa 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -650,17 +650,6 @@ subgroup_packets_to_build (struct update_subgroup *subgrp) return 0; } -static void -bgp_info_addpath_tx_str (int addpath_encode, u_int32_t addpath_tx_id, - char *buf) -{ - buf[0] = '\0'; - if (addpath_encode) - sprintf(buf, " with addpath ID %d", addpath_tx_id); - else - buf[0] = '\0'; -} - /* Make BGP update packet. */ struct bpacket * subgroup_update_packet (struct update_subgroup *subgrp) @@ -1051,11 +1040,21 @@ subgroup_default_update_packet (struct update_subgroup *subgrp, { char attrstr[BUFSIZ]; char buf[PREFIX_STRLEN]; + /* ' with addpath ID ' 17 + * max strlen of uint32 + 10 + * +/- (in case of idiocy) + 1 + * null terminator + 1 + * ============================ 29 */ char tx_id_buf[30]; + attrstr[0] = '\0'; bgp_dump_attr (peer, attr, attrstr, BUFSIZ); - bgp_info_addpath_tx_str (addpath_encode, BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE, tx_id_buf); + + if (addpath_encode) + snprintf(tx_id_buf, sizeof (tx_id_buf), " with addpath ID %u", + BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE); + zlog_debug ("u%" PRIu64 ":s%" PRIu64 " send UPDATE %s%s %s", (SUBGRP_UPDGRP (subgrp))->id, subgrp->id, prefix2str (&p, buf, sizeof (buf)), @@ -1125,9 +1124,17 @@ subgroup_default_withdraw_packet (struct update_subgroup *subgrp) if (bgp_debug_update(NULL, &p, subgrp->update_group, 0)) { char buf[PREFIX_STRLEN]; - char tx_id_buf[INET6_BUFSIZ]; + /* ' with addpath ID ' 17 + * max strlen of uint32 + 10 + * +/- (in case of idiocy) + 1 + * null terminator + 1 + * ============================ 29 */ + char tx_id_buf[30]; + + if (addpath_encode) + snprintf(tx_id_buf, sizeof (tx_id_buf), " with addpath ID %u", + BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE); - bgp_info_addpath_tx_str (addpath_encode, BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE, tx_id_buf); zlog_debug ("u%" PRIu64 ":s%" PRIu64 " send UPDATE %s%s -- unreachable", (SUBGRP_UPDGRP (subgrp))->id, subgrp->id, prefix2str (&p, buf, sizeof (buf)), tx_id_buf); -- 2.39.5