bgpd: fix addpath buffer overrun

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
This commit is contained in:
Quentin Young 2017-06-02 20:53:37 +00:00
parent cb63fd542a
commit 406f99f81d
2 changed files with 28 additions and 18 deletions

View File

@ -40,8 +40,6 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
#include "bgpd/bgp_updgrp.h" #include "bgpd/bgp_updgrp.h"
#include "bgpd/bgp_mplsvpn.h" #include "bgpd/bgp_mplsvpn.h"
#define BGP_ADDPATH_STR 20
unsigned long conf_bgp_debug_as4; unsigned long conf_bgp_debug_as4;
unsigned long conf_bgp_debug_neighbor_events; unsigned long conf_bgp_debug_neighbor_events;
unsigned long conf_bgp_debug_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 rd_buf[RD_ADDRSTRLEN];
char pfx_buf[PREFIX_STRLEN]; 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) if (size < BGP_PRD_PATH_STRLEN)
return NULL; 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. */ /* Note: Path-id is created by default, but only included in update sometimes. */
pathid_buf[0] = '\0'; pathid_buf[0] = '\0';
if (addpath_valid) 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) if (prd)
snprintf (str, size, "RD %s %s%s", snprintf (str, size, "RD %s %s%s",

View File

@ -650,17 +650,6 @@ subgroup_packets_to_build (struct update_subgroup *subgrp)
return 0; 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. */ /* Make BGP update packet. */
struct bpacket * struct bpacket *
subgroup_update_packet (struct update_subgroup *subgrp) subgroup_update_packet (struct update_subgroup *subgrp)
@ -1051,11 +1040,21 @@ subgroup_default_update_packet (struct update_subgroup *subgrp,
{ {
char attrstr[BUFSIZ]; char attrstr[BUFSIZ];
char buf[PREFIX_STRLEN]; 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]; char tx_id_buf[30];
attrstr[0] = '\0'; attrstr[0] = '\0';
bgp_dump_attr (peer, attr, attrstr, BUFSIZ); 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", zlog_debug ("u%" PRIu64 ":s%" PRIu64 " send UPDATE %s%s %s",
(SUBGRP_UPDGRP (subgrp))->id, subgrp->id, (SUBGRP_UPDGRP (subgrp))->id, subgrp->id,
prefix2str (&p, buf, sizeof (buf)), 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)) if (bgp_debug_update(NULL, &p, subgrp->update_group, 0))
{ {
char buf[PREFIX_STRLEN]; 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", zlog_debug ("u%" PRIu64 ":s%" PRIu64 " send UPDATE %s%s -- unreachable",
(SUBGRP_UPDGRP (subgrp))->id, subgrp->id, (SUBGRP_UPDGRP (subgrp))->id, subgrp->id,
prefix2str (&p, buf, sizeof (buf)), tx_id_buf); prefix2str (&p, buf, sizeof (buf)), tx_id_buf);