From 8054cf970b5b56252c458593328761d54ae77483 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:00:25 +0000 Subject: [PATCH 01/21] zebra: fix unused variable on OmniOS Signed-off-by: Quentin Young --- zebra/kernel_socket.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 5f4bd3bbc6..163e01db02 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -304,12 +304,13 @@ size_t rta_getattr(caddr_t sap, void *destp, size_t destlen) size_t rta_getsdlname(caddr_t sap, void *destp, short *destlen) { struct sockaddr_dl *sdl = (struct sockaddr_dl *)sap; - struct sockaddr *sa = (struct sockaddr *)sap; uint8_t *dest = destp; size_t tlen, copylen; copylen = sdl->sdl_nlen; #ifdef HAVE_STRUCT_SOCKADDR_SA_LEN + struct sockaddr *sa = (struct sockaddr *)sap; + tlen = (sa->sa_len == 0) ? sizeof(ROUNDUP_TYPE) : ROUNDUP(sa->sa_len); #else /* !HAVE_STRUCT_SOCKADDR_SA_LEN */ tlen = SAROUNDUP(sap); From ee74220baf60f260795157f08c7c94c22fd225e7 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:01:56 +0000 Subject: [PATCH 02/21] zebra: fix maybe-uninitialized pointer Signed-off-by: Quentin Young --- zebra/kernel_socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 163e01db02..156ce50725 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -523,7 +523,7 @@ static enum zebra_link_type sdl_to_zebra_link_type(unsigned int sdlt) int ifm_read(struct if_msghdr *ifm) { struct interface *ifp = NULL; - struct sockaddr_dl *sdl; + struct sockaddr_dl *sdl = NULL; char ifname[IFNAMSIZ]; short ifnlen = 0; int maskbit; From aa0e96ed1c148aa5d5d91ba64a1ecb73282190a8 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:18:01 +0000 Subject: [PATCH 03/21] lib: fix uninitialized variable in backtrace output Signed-off-by: Quentin Young --- lib/log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/log.c b/lib/log.c index e64c00186b..5e3064a8d8 100644 --- a/lib/log.c +++ b/lib/log.c @@ -602,6 +602,8 @@ void zlog_backtrace_sigsafe(int priority, void *program_counter) backtrace_symbols_fd(array, size, FD); \ } #elif defined(HAVE_PRINTSTACK) + size = 0; + #define DUMP(FD) \ { \ if (program_counter) \ From 53a394720e0f7e0495291a9d828879eebd4ac167 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:20:16 +0000 Subject: [PATCH 04/21] lib: fix false compiler warning Signed-off-by: Quentin Young --- lib/table.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/table.h b/lib/table.h index 14be7ab656..eefd992546 100644 --- a/lib/table.h +++ b/lib/table.h @@ -298,6 +298,8 @@ static inline struct route_node *route_table_iter_next(route_table_iter_t *iter) return NULL; default: + /* Suppress uninitialized variable warning */ + node = NULL; assert(0); } From db878db01a880422cf1525b3fb628407463bcee0 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:22:56 +0000 Subject: [PATCH 05/21] bgpd: fix false compiler warning Signed-off-by: Quentin Young --- bgpd/bgp_packet.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index b5934fb56e..c37e125f53 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2299,6 +2299,8 @@ int bgp_process_packet(struct thread *thread) __FUNCTION__, peer->host); break; default: + /* Suppress uninitialized variable warning */ + mprc = 0; /* * The message type should have been sanitized before * we ever got here. Receipt of a message with an From 21fe4510f9fc7198ac3b4890deb5b8b0eba39dd6 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 18:24:25 +0000 Subject: [PATCH 06/21] bgpd: fix rfapi false compiler warning Signed-off-by: Quentin Young --- bgpd/rfapi/rfapi_import.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index ad0900c2b8..b6d32d36ea 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -4111,6 +4111,9 @@ static void rfapiProcessPeerDownRt(struct peer *peer, timer_service_func = rfapiWithdrawTimerEncap; break; default: + /* Suppress uninitialized variable warning */ + rt = NULL; + timer_service_func = NULL; assert(0); } From 552d6491f08de30b97af876ab923aea10f1b0a1c Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:05:06 +0000 Subject: [PATCH 07/21] bgpd: remove strcpy, strcat Replace with strlcpy, strlcat Signed-off-by: Quentin Young --- bgpd/bgp_community.c | 47 +++++++++++++++----------------------------- bgpd/bgp_packet.c | 23 +++++++++++++--------- bgpd/bgp_routemap.c | 8 ++++---- 3 files changed, 34 insertions(+), 44 deletions(-) diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index 82762072df..c9d36749e2 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -308,12 +308,11 @@ static void set_community_string(struct community *com, bool make_json) if (first) first = 0; else - *pnt++ = ' '; + strlcat(str, " ", len); switch (comval) { case COMMUNITY_INTERNET: - strcpy(pnt, "internet"); - pnt += strlen("internet"); + strlcat(str, "internet", len); if (make_json) { json_string = json_object_new_string("internet"); @@ -322,8 +321,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_GSHUT: - strcpy(pnt, "graceful-shutdown"); - pnt += strlen("graceful-shutdown"); + strlcat(pnt, "graceful-shutdown", len); if (make_json) { json_string = json_object_new_string( "gracefulShutdown"); @@ -332,8 +330,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ACCEPT_OWN: - strcpy(pnt, "accept-own"); - pnt += strlen("accept-own"); + strlcat(pnt, "accept-own", len); if (make_json) { json_string = json_object_new_string( "acceptown"); @@ -342,8 +339,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_TRANSLATED_v4: - strcpy(pnt, "route-filter-translated-v4"); - pnt += strlen("route-filter-translated-v4"); + strlcat(pnt, "route-filter-translated-v4", len); if (make_json) { json_string = json_object_new_string( "routeFilterTranslatedV4"); @@ -352,8 +348,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_v4: - strcpy(pnt, "route-filter-v4"); - pnt += strlen("route-filter-v4"); + strlcat(pnt, "route-filter-v4", len); if (make_json) { json_string = json_object_new_string( "routeFilterV4"); @@ -362,8 +357,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_TRANSLATED_v6: - strcpy(pnt, "route-filter-translated-v6"); - pnt += strlen("route-filter-translated-v6"); + strlcat(pnt, "route-filter-translated-v6", len); if (make_json) { json_string = json_object_new_string( "routeFilterTranslatedV6"); @@ -372,8 +366,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_v6: - strcpy(pnt, "route-filter-v6"); - pnt += strlen("route-filter-v6"); + strlcat(pnt, "route-filter-v6", len); if (make_json) { json_string = json_object_new_string( "routeFilterV6"); @@ -382,8 +375,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_LLGR_STALE: - strcpy(pnt, "llgr-stale"); - pnt += strlen("llgr-stale"); + strlcat(pnt, "llgr-stale", len); if (make_json) { json_string = json_object_new_string( "llgrStale"); @@ -392,8 +384,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_LLGR: - strcpy(pnt, "no-llgr"); - pnt += strlen("no-llgr"); + strlcat(pnt, "no-llgr", len); if (make_json) { json_string = json_object_new_string( "noLlgr"); @@ -402,8 +393,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ACCEPT_OWN_NEXTHOP: - strcpy(pnt, "accept-own-nexthop"); - pnt += strlen("accept-own-nexthop"); + strlcat(pnt, "accept-own-nexthop", len); if (make_json) { json_string = json_object_new_string( "acceptownnexthop"); @@ -412,8 +402,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_BLACKHOLE: - strcpy(pnt, "blackhole"); - pnt += strlen("blackhole"); + strlcat(pnt, "blackhole", len); if (make_json) { json_string = json_object_new_string( "blackhole"); @@ -422,8 +411,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_EXPORT: - strcpy(pnt, "no-export"); - pnt += strlen("no-export"); + strlcat(pnt, "no-export", len); if (make_json) { json_string = json_object_new_string("noExport"); @@ -432,8 +420,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_ADVERTISE: - strcpy(pnt, "no-advertise"); - pnt += strlen("no-advertise"); + strlcat(pnt, "no-advertise", len); if (make_json) { json_string = json_object_new_string("noAdvertise"); @@ -442,8 +429,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_LOCAL_AS: - strcpy(pnt, "local-AS"); - pnt += strlen("local-AS"); + strlcat(pnt, "local-AS", len); if (make_json) { json_string = json_object_new_string("localAs"); json_object_array_add(json_community_list, @@ -451,8 +437,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_PEER: - strcpy(pnt, "no-peer"); - pnt += strlen("no-peer"); + strlcat(pnt, "no-peer", len); if (make_json) { json_string = json_object_new_string("noPeer"); json_object_array_add(json_community_list, diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index c37e125f53..de7df1d47f 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -709,12 +709,15 @@ void bgp_notify_send_with_data(struct peer *peer, uint8_t code, XMALLOC(MTYPE_TMP, bgp_notify.length * 3); for (i = 0; i < bgp_notify.length; i++) if (first) { - sprintf(c, " %02x", data[i]); - strcat(bgp_notify.data, c); + snprintf(c, sizeof(c), " %02x", + data[i]); + strlcat(bgp_notify.data, c, + bgp_notify.length); } else { first = 1; - sprintf(c, "%02x", data[i]); - strcpy(bgp_notify.data, c); + snprintf(c, sizeof(c), "%02x", data[i]); + strlcpy(bgp_notify.data, c, + bgp_notify.length); } } bgp_notify_print(peer, &bgp_notify, "sending"); @@ -1700,14 +1703,16 @@ static int bgp_notify_receive(struct peer *peer, bgp_size_t size) XMALLOC(MTYPE_TMP, bgp_notify.length * 3); for (i = 0; i < bgp_notify.length; i++) if (first) { - sprintf(c, " %02x", + snprintf(c, sizeof(c), " %02x", stream_getc(peer->curr)); - strcat(bgp_notify.data, c); + strlcat(bgp_notify.data, c, + bgp_notify.length); } else { first = 1; - sprintf(c, "%02x", - stream_getc(peer->curr)); - strcpy(bgp_notify.data, c); + snprintf(c, sizeof(c), "%02x", + stream_getc(peer->curr)); + strlcpy(bgp_notify.data, c, + bgp_notify.length); } bgp_notify.raw_data = (uint8_t *)peer->notify.data; } diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index b0ae9d78d1..c8386e6cbe 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -4222,10 +4222,10 @@ DEFUN (set_community, str = community_str(com, false); if (additive) { - argstr = XCALLOC(MTYPE_TMP, - strlen(str) + strlen(" additive") + 1); - strcpy(argstr, str); - strcpy(argstr + strlen(str), " additive"); + size_t argstr_sz = strlen(str) + strlen(" additive") + 1; + argstr = XCALLOC(MTYPE_TMP, argstr_sz); + strlcpy(argstr, str, argstr_sz); + strlcat(argstr, " additive", argstr_sz); ret = generic_set_add(vty, VTY_GET_CONTEXT(route_map_index), "community", argstr); XFREE(MTYPE_TMP, argstr); From 9f73d2c9b60e551be0229790aeb4e4224d9b2c1a Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:05:20 +0000 Subject: [PATCH 08/21] lib: remove some strcpy, strcat Replace with strlcpy, strlcat Signed-off-by: Quentin Young --- lib/command.c | 8 ++++---- lib/libfrr.c | 4 ++-- lib/prefix.c | 2 +- lib/vty.c | 16 +++++++++------- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/command.c b/lib/command.c index 18426e0c51..29f41a712c 100644 --- a/lib/command.c +++ b/lib/command.c @@ -1760,10 +1760,10 @@ static int file_write_config(struct vty *vty) dirfd = open(".", O_DIRECTORY | O_RDONLY); /* if dirfd is invalid, directory sync fails, but we're still OK */ - config_file_sav = XMALLOC( - MTYPE_TMP, strlen(config_file) + strlen(CONF_BACKUP_EXT) + 1); - strcpy(config_file_sav, config_file); - strcat(config_file_sav, CONF_BACKUP_EXT); + size_t config_file_sav_sz = strlen(config_file) + strlen(CONF_BACKUP_EXT) + 1; + config_file_sav = XMALLOC(MTYPE_TMP, config_file_sav_sz); + strlcpy(config_file_sav, config_file, config_file_sav_sz); + strlcat(config_file_sav, CONF_BACKUP_EXT, config_file_sav_sz); config_file_tmp = XMALLOC(MTYPE_TMP, strlen(config_file) + 8); diff --git a/lib/libfrr.c b/lib/libfrr.c index 5970e70a6b..26bdcc1a36 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -80,8 +80,8 @@ static void opt_extend(const struct optspec *os) { const struct option *lo; - strcat(comb_optstr, os->optstr); - strcat(comb_helpstr, os->helpstr); + strlcat(comb_optstr, os->optstr, sizeof(comb_optstr)); + strlcat(comb_helpstr, os->helpstr, sizeof(comb_optstr)); for (lo = os->longopts; lo->name; lo++) memcpy(comb_next_lo++, lo, sizeof(*lo)); } diff --git a/lib/prefix.c b/lib/prefix.c index d2a4c3a432..42d202ddbc 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1365,7 +1365,7 @@ void prefix_mcast_inet4_dump(const char *onfail, struct in_addr addr, int save_errno = errno; if (addr.s_addr == INADDR_ANY) - strcpy(buf, "*"); + strlcpy(buf, "*", buf_size); else { if (!inet_ntop(AF_INET, &addr, buf, buf_size)) { if (onfail) diff --git a/lib/vty.c b/lib/vty.c index 0ee9b78b91..91ba0a43c8 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -1659,7 +1659,7 @@ static struct vty *vty_create(int vty_sock, union sockunion *su) /* configurable parameters not part of basic init */ vty->v_timeout = vty_timeout_val; - strcpy(vty->address, buf); + strlcpy(vty->address, buf, sizeof(vty->address)); if (no_password_check) { if (host.advanced) vty->node = ENABLE_NODE; @@ -1795,7 +1795,7 @@ struct vty *vty_stdio(void (*atclose)(int isexit)) */ vty->node = ENABLE_NODE; vty->v_timeout = 0; - strcpy(vty->address, "console"); + strlcpy(vty->address, "console", sizeof(vty->address)); vty_stdio_resume(); return vty; @@ -2384,9 +2384,10 @@ static FILE *vty_use_backup_config(const char *fullpath) int c; char buffer[512]; - fullpath_sav = malloc(strlen(fullpath) + strlen(CONF_BACKUP_EXT) + 1); - strcpy(fullpath_sav, fullpath); - strcat(fullpath_sav, CONF_BACKUP_EXT); + size_t fullpath_sav_sz = strlen(fullpath) + strlen(CONF_BACKUP_EXT) + 1; + fullpath_sav = malloc(fullpath_sav_sz); + strlcpy(fullpath_sav, fullpath, fullpath_sav_sz); + strlcat(fullpath_sav, CONF_BACKUP_EXT, fullpath_sav_sz); sav = open(fullpath_sav, O_RDONLY); if (sav < 0) { @@ -3079,8 +3080,9 @@ static void vty_save_cwd(void) } } - vty_cwd = XMALLOC(MTYPE_TMP, strlen(cwd) + 1); - strcpy(vty_cwd, cwd); + size_t vty_cwd_sz = strlen(cwd) + 1; + vty_cwd = XMALLOC(MTYPE_TMP, vty_cwd_sz); + strlcpy(vty_cwd, cwd, vty_cwd_sz); } char *vty_get_cwd(void) From 9736ba9e1f57756d0be02dca7c49c7a8e96f0cb9 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:11:22 +0000 Subject: [PATCH 09/21] ripngd: strcat -> strlcat Signed-off-by: Quentin Young --- ripngd/ripngd.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index 411689a7a7..71bc43049a 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -2011,26 +2011,26 @@ static char *ripng_route_subtype_print(struct ripng_info *rinfo) memset(str, 0, 3); if (rinfo->suppress) - strcat(str, "S"); + strlcat(str, "S", sizeof(str)); switch (rinfo->sub_type) { case RIPNG_ROUTE_RTE: - strcat(str, "n"); + strlcat(str, "n", sizeof(str)); break; case RIPNG_ROUTE_STATIC: - strcat(str, "s"); + strlcat(str, "s", sizeof(str)); break; case RIPNG_ROUTE_DEFAULT: - strcat(str, "d"); + strlcat(str, "d", sizeof(str)); break; case RIPNG_ROUTE_REDISTRIBUTE: - strcat(str, "r"); + strlcat(str, "r", sizeof(str)); break; case RIPNG_ROUTE_INTERFACE: - strcat(str, "i"); + strlcat(str, "i", sizeof(str)); break; default: - strcat(str, "?"); + strlcat(str, "?", sizeof(str)); break; } From c35b7e6bea0d4d7e0a080c5024fe49e3793c8c84 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:26:15 +0000 Subject: [PATCH 10/21] pimd: strcpy -> strlcpy Signed-off-by: Quentin Young --- pimd/pim_cmd.c | 92 ++++++++++++++++++++++----------------------- pimd/pim_msdp.c | 2 +- pimd/pim_upstream.c | 15 ++++---- pimd/pim_upstream.h | 3 +- 4 files changed, 57 insertions(+), 55 deletions(-) diff --git a/pimd/pim_cmd.c b/pimd/pim_cmd.c index a2357067f9..881c7a107e 100644 --- a/pimd/pim_cmd.c +++ b/pimd/pim_cmd.c @@ -2001,9 +2001,9 @@ static void pim_show_state(struct pim_instance *pim, struct vty *vty, ifp_in = pim_if_find_by_vif_index(pim, c_oil->oil.mfcc_parent); if (ifp_in) - strcpy(in_ifname, ifp_in->name); + strlcpy(in_ifname, ifp_in->name, sizeof(in_ifname)); else - strcpy(in_ifname, ""); + strlcpy(in_ifname, "", sizeof(in_ifname)); if (src_or_group) { if (strcmp(src_or_group, src_str) @@ -2085,9 +2085,9 @@ static void pim_show_state(struct pim_instance *pim, struct vty *vty, now - c_oil->oif_creation[oif_vif_index]); if (ifp_out) - strcpy(out_ifname, ifp_out->name); + strlcpy(out_ifname, ifp_out->name, sizeof(out_ifname)); else - strcpy(out_ifname, ""); + strlcpy(out_ifname, "", sizeof(out_ifname)); if (uj) { json_ifp_out = json_object_new_object(); @@ -2366,37 +2366,37 @@ static void json_object_pim_upstream_add(json_object *json, static const char * pim_upstream_state2brief_str(enum pim_upstream_state join_state, - char *state_str) + char *state_str, size_t state_str_len) { switch (join_state) { case PIM_UPSTREAM_NOTJOINED: - strcpy(state_str, "NotJ"); + strlcpy(state_str, "NotJ", state_str_len); break; case PIM_UPSTREAM_JOINED: - strcpy(state_str, "J"); + strlcpy(state_str, "J", state_str_len); break; default: - strcpy(state_str, "Unk"); + strlcpy(state_str, "Unk", state_str_len); } return state_str; } static const char *pim_reg_state2brief_str(enum pim_reg_state reg_state, - char *state_str) + char *state_str, size_t state_str_len) { switch (reg_state) { case PIM_REG_NOINFO: - strcpy(state_str, "RegNI"); + strlcpy(state_str, "RegNI", state_str_len); break; case PIM_REG_JOIN: - strcpy(state_str, "RegJ"); + strlcpy(state_str, "RegJ", state_str_len); break; case PIM_REG_JOIN_PENDING: case PIM_REG_PRUNE: - strcpy(state_str, "RegP"); + strlcpy(state_str, "RegP", state_str_len); break; default: - strcpy(state_str, "Unk"); + strlcpy(state_str, "Unk", state_str_len); } return state_str; } @@ -2464,13 +2464,13 @@ static void pim_show_upstream(struct pim_instance *pim, struct vty *vty, pim_time_timer_to_hhmmss(msdp_reg_timer, sizeof(msdp_reg_timer), up->t_msdp_reg_timer); - pim_upstream_state2brief_str(up->join_state, state_str); + pim_upstream_state2brief_str(up->join_state, state_str, sizeof(state_str)); if (up->reg_state != PIM_REG_NOINFO) { char tmp_str[PIM_REG_STATE_STR_LEN]; sprintf(state_str + strlen(state_str), ",%s", - pim_reg_state2brief_str(up->reg_state, - tmp_str)); + pim_reg_state2brief_str(up->reg_state, tmp_str, + sizeof(tmp_str))); } if (uj) { @@ -2521,7 +2521,7 @@ static void pim_show_upstream(struct pim_instance *pim, struct vty *vty, pim_upstream_state2str(up->join_state)); json_object_string_add( json_row, "regState", - pim_reg_state2str(up->reg_state, state_str)); + pim_reg_state2str(up->reg_state, state_str, sizeof(state_str))); json_object_string_add(json_row, "upTime", uptime); json_object_string_add(json_row, "joinTimer", join_timer); @@ -5234,9 +5234,9 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, ifp_in = pim_if_find_by_vif_index(pim, c_oil->oil.mfcc_parent); if (ifp_in) - strcpy(in_ifname, ifp_in->name); + strlcpy(in_ifname, ifp_in->name, sizeof(in_ifname)); else - strcpy(in_ifname, ""); + strlcpy(in_ifname, "", sizeof(in_ifname)); if (uj) { @@ -5291,9 +5291,9 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, found_oif = 1; if (ifp_out) - strcpy(out_ifname, ifp_out->name); + strlcpy(out_ifname, ifp_out->name, sizeof(out_ifname)); else - strcpy(out_ifname, ""); + strlcpy(out_ifname, "", sizeof(out_ifname)); if (uj) { json_ifp_out = json_object_new_object(); @@ -5351,27 +5351,27 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, } else { if (c_oil->oif_flags[oif_vif_index] & PIM_OIF_FLAG_PROTO_PIM) { - strcpy(proto, "PIM"); + strlcpy(proto, "PIM", sizeof(proto)); } if (c_oil->oif_flags[oif_vif_index] & PIM_OIF_FLAG_PROTO_IGMP) { - strcpy(proto, "IGMP"); + strlcpy(proto, "IGMP", sizeof(proto)); } if (c_oil->oif_flags[oif_vif_index] & PIM_OIF_FLAG_PROTO_VXLAN) { - strcpy(proto, "VxLAN"); + strlcpy(proto, "VxLAN", sizeof(proto)); } if (c_oil->oif_flags[oif_vif_index] & PIM_OIF_FLAG_PROTO_SOURCE) { - strcpy(proto, "SRC"); + strlcpy(proto, "SRC", sizeof(proto)); } if (c_oil->oif_flags[oif_vif_index] & PIM_OIF_FLAG_PROTO_STAR) { - strcpy(proto, "STAR"); + strlcpy(proto, "STAR", sizeof(proto)); } vty_out(vty, @@ -5410,9 +5410,9 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, found_oif = 0; if (ifp_in) - strcpy(in_ifname, ifp_in->name); + strlcpy(in_ifname, ifp_in->name, sizeof(in_ifname)); else - strcpy(in_ifname, ""); + strlcpy(in_ifname, "", sizeof(in_ifname)); if (uj) { @@ -5439,7 +5439,7 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, json_object_string_add(json_source, "iif", in_ifname); json_oil = NULL; } else { - strcpy(proto, "STATIC"); + strlcpy(proto, "STATIC", sizeof(proto)); } for (oif_vif_index = 0; oif_vif_index < MAXVIFS; @@ -5461,9 +5461,9 @@ static void show_mroute(struct pim_instance *pim, struct vty *vty, found_oif = 1; if (ifp_out) - strcpy(out_ifname, ifp_out->name); + strlcpy(out_ifname, ifp_out->name, sizeof(out_ifname)); else - strcpy(out_ifname, ""); + strlcpy(out_ifname, "", sizeof(out_ifname)); if (uj) { json_ifp_out = json_object_new_object(); @@ -8998,7 +8998,7 @@ static void ip_msdp_show_peers(struct pim_instance *pim, struct vty *vty, pim_time_uptime(timebuf, sizeof(timebuf), now - mp->uptime); } else { - strcpy(timebuf, "-"); + strlcpy(timebuf, "-", sizeof(timebuf)); } pim_inet4_dump("", mp->peer, peer_str, sizeof(peer_str)); pim_inet4_dump("", mp->local, local_str, @@ -9055,7 +9055,7 @@ static void ip_msdp_show_peers_detail(struct pim_instance *pim, struct vty *vty, pim_time_uptime(timebuf, sizeof(timebuf), now - mp->uptime); } else { - strcpy(timebuf, "-"); + strlcpy(timebuf, "-", sizeof(timebuf)); } pim_inet4_dump("", mp->local, local_str, sizeof(local_str)); @@ -9234,18 +9234,18 @@ static void ip_msdp_show_sa(struct pim_instance *pim, struct vty *vty, bool uj) if (sa->flags & PIM_MSDP_SAF_PEER) { pim_inet4_dump("", sa->rp, rp_str, sizeof(rp_str)); if (sa->up) { - strcpy(spt_str, "yes"); + strlcpy(spt_str, "yes", sizeof(spt_str)); } else { - strcpy(spt_str, "no"); + strlcpy(spt_str, "no", sizeof(spt_str)); } } else { - strcpy(rp_str, "-"); - strcpy(spt_str, "-"); + strlcpy(rp_str, "-", sizeof(rp_str)); + strlcpy(spt_str, "-", sizeof(spt_str)); } if (sa->flags & PIM_MSDP_SAF_LOCAL) { - strcpy(local_str, "yes"); + strlcpy(local_str, "yes", sizeof(local_str)); } else { - strcpy(local_str, "no"); + strlcpy(local_str, "no", sizeof(local_str)); } if (uj) { json_object_object_get_ex(json, grp_str, &json_group); @@ -9299,19 +9299,19 @@ static void ip_msdp_show_sa_entry_detail(struct pim_msdp_sa *sa, pim_inet4_dump("", sa->rp, rp_str, sizeof(rp_str)); pim_inet4_dump("", sa->peer, peer_str, sizeof(peer_str)); if (sa->up) { - strcpy(spt_str, "yes"); + strlcpy(spt_str, "yes", sizeof(spt_str)); } else { - strcpy(spt_str, "no"); + strlcpy(spt_str, "no", sizeof(spt_str)); } } else { - strcpy(rp_str, "-"); - strcpy(peer_str, "-"); - strcpy(spt_str, "-"); + strlcpy(rp_str, "-", sizeof(rp_str)); + strlcpy(peer_str, "-", sizeof(peer_str)); + strlcpy(spt_str, "-", sizeof(spt_str)); } if (sa->flags & PIM_MSDP_SAF_LOCAL) { - strcpy(local_str, "yes"); + strlcpy(local_str, "yes", sizeof(local_str)); } else { - strcpy(local_str, "no"); + strlcpy(local_str, "no", sizeof(local_str)); } pim_time_timer_to_hhmmss(statetimer, sizeof(statetimer), sa->sa_state_timer); diff --git a/pimd/pim_msdp.c b/pimd/pim_msdp.c index 3287e13719..74a3a9836b 100644 --- a/pimd/pim_msdp.c +++ b/pimd/pim_msdp.c @@ -1078,7 +1078,7 @@ static enum pim_msdp_err pim_msdp_peer_new(struct pim_instance *pim, mp->mesh_group_name = XSTRDUP(MTYPE_PIM_MSDP_MG_NAME, mesh_group_name); mp->state = PIM_MSDP_INACTIVE; mp->fd = -1; - strcpy(mp->last_reset, "-"); + strlcpy(mp->last_reset, "-", sizeof(mp->last_reset)); /* higher IP address is listener */ if (ntohl(mp->local.s_addr) > ntohl(mp->peer.s_addr)) { mp->flags |= PIM_MSDP_PEERF_LISTENER; diff --git a/pimd/pim_upstream.c b/pimd/pim_upstream.c index a823962b23..44b8ecbfea 100644 --- a/pimd/pim_upstream.c +++ b/pimd/pim_upstream.c @@ -1399,23 +1399,24 @@ const char *pim_upstream_state2str(enum pim_upstream_state join_state) return "Unknown"; } -const char *pim_reg_state2str(enum pim_reg_state reg_state, char *state_str) +const char *pim_reg_state2str(enum pim_reg_state reg_state, char *state_str, + size_t state_str_len) { switch (reg_state) { case PIM_REG_NOINFO: - strcpy(state_str, "RegNoInfo"); + strlcpy(state_str, "RegNoInfo", state_str_len); break; case PIM_REG_JOIN: - strcpy(state_str, "RegJoined"); + strlcpy(state_str, "RegJoined", state_str_len); break; case PIM_REG_JOIN_PENDING: - strcpy(state_str, "RegJoinPend"); + strlcpy(state_str, "RegJoinPend", state_str_len); break; case PIM_REG_PRUNE: - strcpy(state_str, "RegPrune"); + strlcpy(state_str, "RegPrune", state_str_len); break; default: - strcpy(state_str, "RegUnknown"); + strlcpy(state_str, "RegUnknown", state_str_len); } return state_str; } @@ -1432,7 +1433,7 @@ static int pim_upstream_register_stop_timer(struct thread *t) char state_str[PIM_REG_STATE_STR_LEN]; zlog_debug("%s: (S,G)=%s[%s] upstream register stop timer %s", __PRETTY_FUNCTION__, up->sg_str, pim->vrf->name, - pim_reg_state2str(up->reg_state, state_str)); + pim_reg_state2str(up->reg_state, state_str, sizeof(state_str))); } switch (up->reg_state) { diff --git a/pimd/pim_upstream.h b/pimd/pim_upstream.h index 102826ac71..02ae998290 100644 --- a/pimd/pim_upstream.h +++ b/pimd/pim_upstream.h @@ -283,7 +283,8 @@ void pim_upstream_switch(struct pim_instance *pim, struct pim_upstream *up, const char *pim_upstream_state2str(enum pim_upstream_state join_state); #define PIM_REG_STATE_STR_LEN 12 -const char *pim_reg_state2str(enum pim_reg_state state, char *state_str); +const char *pim_reg_state2str(enum pim_reg_state state, char *state_str, + size_t state_str_len); int pim_upstream_inherited_olist_decide(struct pim_instance *pim, struct pim_upstream *up); From 65b88efa72d22a33cca795899268bcb0eabda979 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:27:33 +0000 Subject: [PATCH 11/21] pbrd: strcpy -> strlcpy Signed-off-by: Quentin Young --- pbrd/pbr_nht.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pbrd/pbr_nht.c b/pbrd/pbr_nht.c index 52506542bc..fc78b8ed1f 100644 --- a/pbrd/pbr_nht.c +++ b/pbrd/pbr_nht.c @@ -191,7 +191,7 @@ static void *pbr_nhgc_alloc(void *p) new = XCALLOC(MTYPE_PBR_NHG, sizeof(*new)); - strcpy(new->name, pnhgc->name); + strlcpy(new->name, pnhgc->name, sizeof(pnhgc->name)); new->table_id = pbr_nht_get_next_tableid(false); DEBUGD(&pbr_dbg_nht, "%s: NHT: %s assigned Table ID: %u", From 2e600d75293e34196e586a9ff96be819dd5ba1f4 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:28:55 +0000 Subject: [PATCH 12/21] vtysh: strcpy -> strlcpy, strcat -> strlcat Signed-off-by: Quentin Young --- vtysh/vtysh.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index a0b119c3eb..51d5e42915 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -681,7 +681,7 @@ int vtysh_mark_file(const char *filename) while (fgets(vty->buf, VTY_BUFSIZ, confp)) { lineno++; tried = 0; - strcpy(vty_buf_copy, vty->buf); + strlcpy(vty_buf_copy, vty->buf, VTY_BUFSIZ); vty_buf_trimmed = trim(vty_buf_copy); switch (vty->node) { @@ -2702,9 +2702,10 @@ static void backup_config_file(const char *fbackup) { char *integrate_sav = NULL; - integrate_sav = malloc(strlen(fbackup) + strlen(CONF_BACKUP_EXT) + 1); - strcpy(integrate_sav, fbackup); - strcat(integrate_sav, CONF_BACKUP_EXT); + size_t integrate_sav_sz = strlen(fbackup) + strlen(CONF_BACKUP_EXT) + 1; + integrate_sav = malloc(integrate_sav_sz); + strlcpy(integrate_sav, fbackup, integrate_sav_sz); + strlcat(integrate_sav, CONF_BACKUP_EXT, integrate_sav_sz); /* Move current configuration file to backup config file. */ if (unlink(integrate_sav) != 0) { From 2dfe5a077573b76ed7f03d5d91c78682854c976c Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 21:30:10 +0000 Subject: [PATCH 13/21] staticd: strcpy -> strlcpy Signed-off-by: Quentin Young --- staticd/static_routes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staticd/static_routes.c b/staticd/static_routes.c index cde31df14f..5f9ecad694 100644 --- a/staticd/static_routes.c +++ b/staticd/static_routes.c @@ -127,7 +127,7 @@ int static_add_route(afi_t afi, safi_t safi, uint8_t type, struct prefix *p, si->tag = tag; si->vrf_id = svrf->vrf->vrf_id; si->nh_vrf_id = nh_svrf->vrf->vrf_id; - strcpy(si->nh_vrfname, nh_svrf->vrf->name); + strlcpy(si->nh_vrfname, nh_svrf->vrf->name, sizeof(si->nh_vrfname)); si->table_id = table_id; si->onlink = onlink; From eab4a5c2d06d6a9b44d6d3333b5c3f2d74e95da4 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Mon, 6 May 2019 22:55:59 +0000 Subject: [PATCH 14/21] zebra: strcat -> strlcat Signed-off-by: Quentin Young --- zebra/rt_netlink.c | 2 +- zebra/zebra_mpls.c | 5 +++-- zebra/zebra_rib.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index def5bf7d88..92c78a4cbb 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -813,7 +813,7 @@ static int netlink_route_change_read_multicast(struct nlmsghdr *h, sprintf(temp, "%s(%d) ", ifp ? ifp->name : "Unknown", oif[count]); - strcat(oif_list, temp); + strlcat(oif_list, temp, sizeof(oif_list)); } struct zebra_vrf *zvrf = zebra_vrf_lookup_by_id(vrf); ifp = if_lookup_by_index(iif, vrf); diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c index 48366417dd..5214f1f22d 100644 --- a/zebra/zebra_mpls.c +++ b/zebra/zebra_mpls.c @@ -1693,8 +1693,9 @@ static char *snhlfe2str(zebra_snhlfe_t *snhlfe, char *buf, int size) case NEXTHOP_TYPE_IPV6_IFINDEX: inet_ntop(AF_INET6, &snhlfe->gate.ipv6, buf, size); if (snhlfe->ifindex) - strcat(buf, - ifindex2ifname(snhlfe->ifindex, VRF_DEFAULT)); + strlcat(buf, + ifindex2ifname(snhlfe->ifindex, VRF_DEFAULT), + size); break; default: break; diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 600e820bc4..b14f8e3cc9 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -129,7 +129,7 @@ _rnode_zlog(const char *_func, vrf_id_t vrf_id, struct route_node *rn, srcdest_rnode2str(rn, buf, sizeof(buf)); if (info->safi == SAFI_MULTICAST) - strcat(buf, " (MRIB)"); + strlcat(buf, " (MRIB)", sizeof(buf)); } else { snprintf(buf, sizeof(buf), "{(route_node *) NULL}"); } From fcb072cdbfa87ffff0a5c94c29217a3235bbe8f3 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Wed, 8 May 2019 18:33:53 +0000 Subject: [PATCH 15/21] lib, zebra: remove uses of strncpy This removes the last removable uses of strncpy in FRR. Signed-off-by: Quentin Young --- lib/command_match.c | 4 ++-- lib/vty.c | 2 +- zebra/ioctl.c | 4 ++-- zebra/ioctl_solaris.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/command_match.c b/lib/command_match.c index 8b34d1e3eb..9456e1585a 100644 --- a/lib/command_match.c +++ b/lib/command_match.c @@ -723,7 +723,7 @@ static enum match_type match_ipv4(const char *str) if (str - sp > 3) return no_match; - strncpy(buf, sp, str - sp); + memcpy(buf, sp, str - sp); if (atoi(buf) > 255) return no_match; @@ -774,7 +774,7 @@ static enum match_type match_ipv4_prefix(const char *str) if (str - sp > 3) return no_match; - strncpy(buf, sp, str - sp); + memcpy(buf, sp, str - sp); if (atoi(buf) > 255) return no_match; diff --git a/lib/vty.c b/lib/vty.c index 91ba0a43c8..9bee7b6c83 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -998,7 +998,7 @@ static void vty_describe_fold(struct vty *vty, int cmd_width, if (pos == 0) break; - strncpy(buf, p, pos); + memcpy(buf, p, pos); buf[pos] = '\0'; vty_out(vty, " %-*s %s\n", cmd_width, cmd, buf); diff --git a/zebra/ioctl.c b/zebra/ioctl.c index 322527015b..8202e076af 100644 --- a/zebra/ioctl.c +++ b/zebra/ioctl.c @@ -245,7 +245,7 @@ static int if_set_prefix_ctx(const struct zebra_dplane_ctx *ctx) p = (struct prefix_ipv4 *)dplane_ctx_get_intf_addr(ctx); memset(&addreq, 0, sizeof(addreq)); - strncpy((char *)&addreq.ifra_name, dplane_ctx_get_ifname(ctx), + strlcpy((char *)&addreq.ifra_name, dplane_ctx_get_ifname(ctx), sizeof(addreq.ifra_name)); memset(&addr, 0, sizeof(struct sockaddr_in)); @@ -296,7 +296,7 @@ static int if_unset_prefix_ctx(const struct zebra_dplane_ctx *ctx) p = (struct prefix_ipv4 *)dplane_ctx_get_intf_addr(ctx); memset(&addreq, 0, sizeof(addreq)); - strncpy((char *)&addreq.ifra_name, dplane_ctx_get_ifname(ctx), + strlcpy((char *)&addreq.ifra_name, dplane_ctx_get_ifname(ctx), sizeof(addreq.ifra_name)); memset(&addr, 0, sizeof(struct sockaddr_in)); diff --git a/zebra/ioctl_solaris.c b/zebra/ioctl_solaris.c index ccfa7a4a4c..1f96fa23ea 100644 --- a/zebra/ioctl_solaris.c +++ b/zebra/ioctl_solaris.c @@ -286,7 +286,7 @@ static int if_unset_prefix_ctx(const struct zebra_dplane_ctx *ctx) p = (struct prefix_ipv4 *)dplane_ctx_get_intf_addr(ctx); - strncpy(ifreq.ifr_name, dplane_ctx_get_ifname(ctx), + strlcpy(ifreq.ifr_name, dplane_ctx_get_ifname(ctx), sizeof(ifreq.ifr_name)); memset(&addr, 0, sizeof(struct sockaddr_in)); From 8da59e56def8884fd5b6a8b788c04f65242a8b0f Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Wed, 8 May 2019 19:17:32 +0000 Subject: [PATCH 16/21] ospfd: initialize maybe-uninitialized bool Signed-off-by: Quentin Young --- ospfd/ospf_te.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c index 1488aa88cd..e683636639 100644 --- a/ospfd/ospf_te.c +++ b/ospfd/ospf_te.c @@ -2535,7 +2535,7 @@ DEFUN (show_ip_ospf_mpls_te_link, struct interface *ifp = NULL; struct listnode *node; char *vrf_name = NULL; - bool all_vrf; + bool all_vrf = false; int inst = 0; int idx_vrf = 0; struct ospf *ospf = NULL; From 5041dc4fbf6be647f1fb36988bf355eb4636dbf9 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Thu, 9 May 2019 16:04:07 +0000 Subject: [PATCH 17/21] bgpd: suppress dead store warning Signed-off-by: Quentin Young --- bgpd/bgp_packet.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index de7df1d47f..655a4745cb 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -2306,6 +2306,7 @@ int bgp_process_packet(struct thread *thread) default: /* Suppress uninitialized variable warning */ mprc = 0; + (void)mprc; /* * The message type should have been sanitized before * we ever got here. Receipt of a message with an From 6e0b62b4281208a260cac7c3521814d84bacbabd Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Wed, 15 May 2019 07:35:04 +0000 Subject: [PATCH 18/21] bgpd: fix pointer bug introduced in strlcat change Inconsistent use of a string pointer led to improperly terminated strings (terminated too soon) Signed-off-by: Quentin Young --- bgpd/bgp_community.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index c9d36749e2..6270a4dd4c 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -205,7 +205,6 @@ static void set_community_string(struct community *com, bool make_json) { int i; char *str; - char *pnt; int len; int first; uint32_t comval; @@ -297,7 +296,7 @@ static void set_community_string(struct community *com, bool make_json) } /* Allocate memory. */ - str = pnt = XMALLOC(MTYPE_COMMUNITY_STR, len); + str = XMALLOC(MTYPE_COMMUNITY_STR, len); first = 1; /* Fill in string. */ @@ -321,7 +320,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_GSHUT: - strlcat(pnt, "graceful-shutdown", len); + strlcat(str, "graceful-shutdown", len); if (make_json) { json_string = json_object_new_string( "gracefulShutdown"); @@ -330,7 +329,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ACCEPT_OWN: - strlcat(pnt, "accept-own", len); + strlcat(str, "accept-own", len); if (make_json) { json_string = json_object_new_string( "acceptown"); @@ -339,7 +338,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_TRANSLATED_v4: - strlcat(pnt, "route-filter-translated-v4", len); + strlcat(str, "route-filter-translated-v4", len); if (make_json) { json_string = json_object_new_string( "routeFilterTranslatedV4"); @@ -348,7 +347,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_v4: - strlcat(pnt, "route-filter-v4", len); + strlcat(str, "route-filter-v4", len); if (make_json) { json_string = json_object_new_string( "routeFilterV4"); @@ -357,7 +356,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_TRANSLATED_v6: - strlcat(pnt, "route-filter-translated-v6", len); + strlcat(str, "route-filter-translated-v6", len); if (make_json) { json_string = json_object_new_string( "routeFilterTranslatedV6"); @@ -366,7 +365,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ROUTE_FILTER_v6: - strlcat(pnt, "route-filter-v6", len); + strlcat(str, "route-filter-v6", len); if (make_json) { json_string = json_object_new_string( "routeFilterV6"); @@ -375,7 +374,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_LLGR_STALE: - strlcat(pnt, "llgr-stale", len); + strlcat(str, "llgr-stale", len); if (make_json) { json_string = json_object_new_string( "llgrStale"); @@ -384,7 +383,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_LLGR: - strlcat(pnt, "no-llgr", len); + strlcat(str, "no-llgr", len); if (make_json) { json_string = json_object_new_string( "noLlgr"); @@ -393,7 +392,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_ACCEPT_OWN_NEXTHOP: - strlcat(pnt, "accept-own-nexthop", len); + strlcat(str, "accept-own-nexthop", len); if (make_json) { json_string = json_object_new_string( "acceptownnexthop"); @@ -402,7 +401,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_BLACKHOLE: - strlcat(pnt, "blackhole", len); + strlcat(str, "blackhole", len); if (make_json) { json_string = json_object_new_string( "blackhole"); @@ -411,7 +410,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_EXPORT: - strlcat(pnt, "no-export", len); + strlcat(str, "no-export", len); if (make_json) { json_string = json_object_new_string("noExport"); @@ -420,7 +419,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_ADVERTISE: - strlcat(pnt, "no-advertise", len); + strlcat(str, "no-advertise", len); if (make_json) { json_string = json_object_new_string("noAdvertise"); @@ -429,7 +428,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_LOCAL_AS: - strlcat(pnt, "local-AS", len); + strlcat(str, "local-AS", len); if (make_json) { json_string = json_object_new_string("localAs"); json_object_array_add(json_community_list, @@ -437,7 +436,7 @@ static void set_community_string(struct community *com, bool make_json) } break; case COMMUNITY_NO_PEER: - strlcat(pnt, "no-peer", len); + strlcat(str, "no-peer", len); if (make_json) { json_string = json_object_new_string("noPeer"); json_object_array_add(json_community_list, @@ -447,17 +446,17 @@ static void set_community_string(struct community *com, bool make_json) default: as = (comval >> 16) & 0xFFFF; val = comval & 0xFFFF; - sprintf(pnt, "%u:%d", as, val); + char buf[32]; + snprintf(buf, sizeof(buf), "%u:%d", as, val); + strlcat(str, buf, len); if (make_json) { - json_string = json_object_new_string(pnt); + json_string = json_object_new_string(buf); json_object_array_add(json_community_list, json_string); } - pnt += strlen(pnt); break; } } - *pnt = '\0'; if (make_json) { json_object_string_add(com->json, "string", str); From daeb97e980ab7e28caf11a15deccc1d36a08042b Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Wed, 15 May 2019 22:42:41 +0000 Subject: [PATCH 19/21] lib: use static storage for vty_cwd Why are we allocating this Signed-off-by: Quentin Young --- lib/vty.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/vty.c b/lib/vty.c index 9bee7b6c83..2d97cca351 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -84,7 +84,7 @@ static char *vty_ipv6_accesslist_name = NULL; static vector Vvty_serv_thread; /* Current directory. */ -char *vty_cwd = NULL; +char vty_cwd[MAXPATHLEN]; /* Login password check. */ static int no_password_check = 0; @@ -3056,10 +3056,9 @@ void vty_reset(void) static void vty_save_cwd(void) { - char cwd[MAXPATHLEN]; char *c; - c = getcwd(cwd, MAXPATHLEN); + c = getcwd(vty_cwd, sizeof(vty_cwd)); if (!c) { /* @@ -3073,16 +3072,12 @@ static void vty_save_cwd(void) SYSCONFDIR, errno); exit(-1); } - if (getcwd(cwd, MAXPATHLEN) == NULL) { + if (getcwd(vty_cwd, sizeof(vty_cwd)) == NULL) { flog_err_sys(EC_LIB_SYSTEM_CALL, "Failure to getcwd, errno: %d", errno); exit(-1); } } - - size_t vty_cwd_sz = strlen(cwd) + 1; - vty_cwd = XMALLOC(MTYPE_TMP, vty_cwd_sz); - strlcpy(vty_cwd, cwd, vty_cwd_sz); } char *vty_get_cwd(void) @@ -3148,7 +3143,7 @@ void vty_init(struct thread_master *master_thread) void vty_terminate(void) { - XFREE(MTYPE_TMP, vty_cwd); + memset(vty_cwd, 0x00, sizeof(vty_cwd)); if (vtyvec && Vvty_serv_thread) { vty_reset(); From f9bff3be9ebff9b0edf27b609e1572388d2b9f16 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Fri, 17 May 2019 00:08:21 +0000 Subject: [PATCH 20/21] bgpd: use XCALLOC to allocate string buf Signed-off-by: Quentin Young --- bgpd/bgp_community.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index 6270a4dd4c..6fc52ff9e0 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -296,7 +296,7 @@ static void set_community_string(struct community *com, bool make_json) } /* Allocate memory. */ - str = XMALLOC(MTYPE_COMMUNITY_STR, len); + str = XCALLOC(MTYPE_COMMUNITY_STR, len); first = 1; /* Fill in string. */ From 67c726a10d90b9edc02e99e5a9064d14f9920309 Mon Sep 17 00:00:00 2001 From: Quentin Young Date: Tue, 28 May 2019 20:25:45 +0000 Subject: [PATCH 21/21] lib: fix helpstring truncation Signed-off-by: Quentin Young --- lib/libfrr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libfrr.c b/lib/libfrr.c index 26bdcc1a36..cd5a164c53 100644 --- a/lib/libfrr.c +++ b/lib/libfrr.c @@ -81,7 +81,7 @@ static void opt_extend(const struct optspec *os) const struct option *lo; strlcat(comb_optstr, os->optstr, sizeof(comb_optstr)); - strlcat(comb_helpstr, os->helpstr, sizeof(comb_optstr)); + strlcat(comb_helpstr, os->helpstr, sizeof(comb_helpstr)); for (lo = os->longopts; lo->name; lo++) memcpy(comb_next_lo++, lo, sizeof(*lo)); }