From edf344ebff93f9ae470ec2a373b5fa65c1442670 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 3 Nov 2017 14:09:24 -0400 Subject: [PATCH 1/2] bgpd: Fix crash with ecommunity string When we are displaying a extended community ECOMMUNITY_SITE_ORIGIN the display sprintf is this: len = sprintf( str_buf + str_pnt, "EVPN:%02x:%02x:%02x:%02x:%02x:%02x", macaddr[0], macaddr[1], macaddr[2], macaddr[3], macaddr[4], macaddr[5]); The problem with this is that macaddr[0] is passed in as a integer so the sprintf function thinks that the value to display is much larger than it actually is. The ECOMMUNITY_STR_DEFAULT_LEN is 27 So the resulting string no-longer fits in memory and we write off the end of the buffer and can crash. If we force the passed in value to be a uint8_t then we get the expected output since a single byte is displayed as 2 hex characters and the resulting string fits in str_buf. Signed-off-by: Donald Sharp --- bgpd/bgp_ecommunity.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c index bdcc12705c..e19f516505 100644 --- a/bgpd/bgp_ecommunity.c +++ b/bgpd/bgp_ecommunity.c @@ -702,8 +702,12 @@ char *ecommunity_ecom2str(struct ecommunity *ecom, int format, int filter) len = sprintf( str_buf + str_pnt, "EVPN:%02x:%02x:%02x:%02x:%02x:%02x", - macaddr[0], macaddr[1], macaddr[2], - macaddr[3], macaddr[4], macaddr[5]); + (uint8_t)macaddr[0], + (uint8_t)macaddr[1], + (uint8_t)macaddr[2], + (uint8_t)macaddr[3], + (uint8_t)macaddr[4], + (uint8_t)macaddr[5]); } else if (*pnt == ECOMMUNITY_EVPN_SUBTYPE_MACMOBILITY) { u_int32_t seqnum; From d2b6417bd6f91cdc614c3bf983370c030f03642b Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 3 Nov 2017 15:25:31 -0400 Subject: [PATCH 2/2] bgpd: Prevent infinite loop when reading capabilities If the user has configured the ability to override the capabilities or if the afi/safi passed as part of the _MP capability is not understood, then we can enter into an infinite loop as part of the capability parsing. Signed-off-by: Donald Sharp --- bgpd/bgp_packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index a66d0590c9..79ce550a38 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2011,6 +2011,7 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, /* Fetch structure to the byte stream. */ memcpy(&mpc, pnt + 3, sizeof(struct capability_mp_data)); + pnt += hdr->length + 3; /* We know MP Capability Code. */ if (hdr->code == CAPABILITY_CODE_MP) { @@ -2063,7 +2064,6 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, "%s unrecognized capability code: %d - ignored", peer->host, hdr->code); } - pnt += hdr->length + 3; } return 0; }