Skip to content

Commit 499504b

Browse files
committed
bgpd: Fix stack overflow when debug printing label information
==11==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f8ec03890b2 WRITE of size 5 at 0x7f8ec03890b2 thread T0 #0 vsnprintf (sanitizer_common_interceptors.inc:1652) #1 snprintf (sanitizer_common_interceptors.inc:1723) #2 mpls_labels2str (bgpd/bgp_label.c:699) #3 bgp_debug_rdpfxpath2str (bgpd/bgp_debug.c:2967) #4 subgroup_update_packet (bgpd/bgp_updgrp_packet.c:880) #5 bgp_generate_updgrp_packets (bgpd/bgp_packet.c:504) [144, 174) 'tag_buf' (line 2918) <== Memory access at offset 178 overflows this variable [208, 238) 'pathid_buf' (line 2928) Reported-by: Qifan Zhang, Palo Alto Networks Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
1 parent 18549aa commit 499504b

5 files changed

Lines changed: 17 additions & 10 deletions

File tree

bgpd/bgp_debug.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2951,7 +2951,7 @@ const char *bgp_debug_rdpfxpath2str(afi_t afi, safi_t safi,
29512951
struct bgp_route_evpn *overlay_index,
29522952
char *str, int size)
29532953
{
2954-
char tag_buf[30];
2954+
char tag_buf[sizeof(" label ") + BGP_MAX_LABEL_DIGITS];
29552955
char overlay_index_buf[INET6_ADDRSTRLEN + 14];
29562956
const struct prefix_evpn *evp;
29572957
int len = 0;

bgpd/bgp_label.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -681,24 +681,30 @@ char *mpls_labels2str(mpls_label_t *labels, uint8_t num_labels, const char *pref
681681
int size)
682682
{
683683
uint32_t label_value;
684-
uint32_t len = 0;
684+
int len = 0;
685+
int ret;
685686
uint8_t i;
686687

688+
if (size <= 0)
689+
return buf;
690+
687691
buf[0] = '\0';
688692
if (!num_labels)
689693
return buf;
690694

691695
if (prefix) {
692-
strlcat(buf + len, prefix, size - len);
696+
strlcat(buf, prefix, size);
693697
len = strlen(buf);
698+
if (len >= size - 1)
699+
return buf;
694700
}
695701

696-
label_value = decode_label(&labels[0]);
697-
len += snprintf(buf + len, size - len, "%u", label_value);
698-
699-
for (i = 1; i < num_labels; i++) {
702+
for (i = 0; i < num_labels; i++) {
700703
label_value = decode_label(&labels[i]);
701-
len += snprintf(buf + len, size - len, "/%u", label_value);
704+
ret = snprintf(buf + len, size - len, "%s%u", i == 0 ? "" : "/", label_value);
705+
if (ret < 0 || ret >= size - len)
706+
return buf;
707+
len += ret;
702708
}
703709

704710
return buf;

bgpd/bgp_label.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ struct peer;
2323
* It is impossible to pass more than 10 labels by BGP-LU
2424
*/
2525
#define BGP_MAX_LABELS 10
26+
#define BGP_MAX_LABEL_DIGITS (9 * BGP_MAX_LABELS)
2627

2728
/* MPLS label(s) - VNI(s) for EVPN-VxLAN */
2829
struct bgp_labels {

bgpd/bgp_route.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12436,7 +12436,7 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn,
1243612436
struct attr *pattr, uint16_t show_opts)
1243712437
{
1243812438
char buf[INET6_ADDRSTRLEN];
12439-
char labels_buf[9 * BGP_MAX_LABELS]; /* 8 per label + / or \0 for each */
12439+
char labels_buf[BGP_MAX_LABEL_DIGITS]; /* 8 per label + / or \0 for each */
1244012440
char vni_buf[30] = {};
1244112441
struct attr *attr = pattr ? pattr : path->attr;
1244212442
time_t tbuf;

bgpd/bgp_zebra.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1545,7 +1545,7 @@ static void bgp_debug_zebra_nh(struct zapi_route *api)
15451545
char eth_buf[ETHER_ADDR_STRLEN + 7] = { '\0' };
15461546
char buf1[ETHER_ADDR_STRLEN];
15471547
/* strlen("label ") + 8 chars per label + '/' or '\0' for each label */
1548-
char label_buf[6 + 9 * BGP_MAX_LABELS];
1548+
char label_buf[6 + BGP_MAX_LABEL_DIGITS];
15491549
char sid_buf[20];
15501550
char segs_buf[256];
15511551
struct zapi_nexthop *api_nh;

0 commit comments

Comments
 (0)