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 <[email protected]>
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;