bgpd: fix addpath buffer overrun

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c
index 232f53c778a7487f59f9f9d9745e9e5e2e2344e4..fb350d51a191ce46b4d8e3a0e4c4e20cd5e6b1da 100644 (file)
--- 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 5872ca6f3f30dd082e88427b528fc554cb0a222f..588f8a66fac06873c55f38d3fa3260d6db851e3e 100644 (file)
--- 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);