Re: [PATCH 4/4] kconfig: prevent out-of-bounds user input for numeric options

[Nathan Chancellor]

> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 4234a51d16fd..2227d89d6328 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -354,6 +354,12 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
>  	case S_INT:
>  	case S_HEX:
>  		if (sym_string_valid(sym, p)) {
> +			if (def != S_DEF_AUTO &&
> +			    !sym_string_check_bounds(sym, p))
> +				/* hex uses 64-bit unsigned integer */
> +				conf_warning("value '%s' for %s is outside the 64-bit %s integer bounds",
> +					     p, sym->name,
> +					     sym->type == S_INT ? "signed" : "unsigned");
>  			sym->def[def].val = xstrdup(p);
>  			sym->flags |= def_flags;
>  		} else {
...
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 2d8b0c65ce1e..6f99216ee76d 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -4,7 +4,6 @@
>   */
>  
>  #include <ctype.h>
> -#include <errno.h>
>  #include <stdarg.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -255,17 +254,13 @@ static int menu_validate_number(struct symbol *sym, struct symbol *sym2,
>  		return 1;
>  	}
>  
> -	errno = 0;
> -	if (sym->type == S_INT) {
> +	if (sym->type == S_INT)
>  		type_bounds = "64-bit signed integer";
> -		strtoll(sym2->name, NULL, 10);
> -	} else {
> +	else
>  		/* hex */
>  		type_bounds = "64-bit unsigned integer";
> -		strtoull(sym2->name, NULL, 16);
> -	}
>  
> -	if (errno == ERANGE) {
> +	if (!sym_string_check_bounds(sym, sym2->name)) {
>  		fprintf(stderr,
>  			"%s:%d: error: %s constant '%s' is outside the %s bounds\n",
>  			prop->filename, prop->lineno, sym_type_name(sym->type),

With this, you could inline the type bounds string like you did above:

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 6f99216ee76d..f4b5b11991b4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -238,8 +238,6 @@ void menu_add_symbol(enum prop_type type, struct symbol *sym, struct expr *dep)
 static int menu_validate_number(struct symbol *sym, struct symbol *sym2,
 				const struct property *prop)
 {
-	const char *type_bounds;
-
 	if (sym->type != S_INT && sym->type != S_HEX)
 		return 0;
 
@@ -254,17 +252,11 @@ static int menu_validate_number(struct symbol *sym, struct symbol *sym2,
 		return 1;
 	}
 
-	if (sym->type == S_INT)
-		type_bounds = "64-bit signed integer";
-	else
-		/* hex */
-		type_bounds = "64-bit unsigned integer";
-
 	if (!sym_string_check_bounds(sym, sym2->name)) {
 		fprintf(stderr,
-			"%s:%d: error: %s constant '%s' is outside the %s bounds\n",
+			"%s:%d: error: %s constant '%s' is outside the 64-bit %s integer bounds\n",
 			prop->filename, prop->lineno, sym_type_name(sym->type),
-			sym2->name, type_bounds);
+			sym2->name, sym->type == S_INT ? "signed" : "unsigned");
 
 		return 1;
 	}

> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 7e81b3676ee9..2d1c021fa395 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -5,6 +5,7 @@
>  
>  #include <sys/types.h>
>  #include <ctype.h>
> +#include <errno.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <regex.h>
> @@ -711,6 +712,21 @@ bool sym_string_valid(struct symbol *sym, const char *str)
>  	}
>  }
>  
> +bool sym_string_check_bounds(struct symbol *sym, const char *str)
> +{
> +	errno = 0;
> +
> +	if (sym->type == S_INT)
> +		strtoll(str, NULL, 10);
> +	else if (sym->type == S_HEX)
> +		strtoull(str, NULL, 16);
> +	else
> +		/* string */
> +		return true;
> +
> +	return errno != ERANGE;
> +}
> +
>  bool sym_string_within_range(struct symbol *sym, const char *str)
>  {
>  	struct property *prop;
> @@ -722,6 +738,8 @@ bool sym_string_within_range(struct symbol *sym, const char *str)
>  	case S_INT:
>  		if (!sym_string_valid(sym, str))
>  			return false;
> +		if (!sym_string_check_bounds(sym, str))
> +			return false;
>  		prop = sym_get_range_prop(sym);
>  		if (!prop)
>  			return true;
> @@ -731,6 +749,8 @@ bool sym_string_within_range(struct symbol *sym, const char *str)
>  	case S_HEX:
>  		if (!sym_string_valid(sym, str))
>  			return false;
> +		if (!sym_string_check_bounds(sym, str))
> +			return false;
>  		prop = sym_get_range_prop(sym);
>  		if (!prop)
>  			return true;

Sashiko has a comment that seems to be relevant here unless I
misunderstand what it is complaining about:

https://sashiko.dev/#/patchset/64934

Otherwise, I like the direction here.

-- 
Cheers,
Nathan