Hi, If I do not mistake, cf_new_symbol() has incorrect check of symbol length. In other places where SYM_MAX_LEN is used, it is expected that leading zero is counted in it. But the check in cf_new_symbol() allows symbol length equal to SYM_MAX_LEN. This does not cause a problem (as I unederstand), because the space is allocated anyway and this string is not copied later to fixed size arrays. I attached a patch with the fix, there is also a couple of bsprintf replaced with safer bsnprintf calls.
commit 9ecf13ebd196d06ce03b7c3fc84d70193cc4cfe9 Author: Alexander Zubkov <gr...@qrator.net> Date: Fri Jan 27 02:35:16 2023 +0100
fix symbol length check, use safer bsnprintf diff --git a/conf/cf-lex.l b/conf/cf-lex.l index ceedee8a..65b5e941 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -582,7 +582,7 @@ cf_new_symbol(const byte *c) struct symbol *s; uint l = strlen(c); - if (l > SYM_MAX_LEN) + if (l >= SYM_MAX_LEN) cf_error("Symbol too long"); cf_swap_soft_scope(); @@ -676,7 +676,7 @@ cf_default_name(char *template, int *counter) for(;;) { - bsprintf(buf, template, ++(*counter)); + bsnprintf(buf, sizeof(buf), template, ++(*counter)); s = cf_get_symbol(buf); if (s->class == SYM_VOID) return s; diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 2e442e16..5b0894b3 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -491,7 +491,8 @@ bgp_spawn(struct bgp_proto *pp, ip_addr remote_ip) struct symbol *sym; char fmt[SYM_MAX_LEN]; - bsprintf(fmt, "%s%%0%dd", pp->cf->dynamic_name, pp->cf->dynamic_name_digits); + /* dynamic_name and _digits values are limited in the config parser */ + bsnprintf(fmt, sizeof(fmt), "%s%%0%dd", pp->cf->dynamic_name, pp->cf->dynamic_name_digits); /* This is hack, we would like to share config, but we need to copy it now */ new_config = config;