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;

Reply via email to