[RFC PATCH 1/1] env: introduce variable ranges

26 Aug 2024

From: Lukas Funke lukas.funke@weidmueller.com
This commit extends the flags-variable with ranges for environment
variables. The range is appended to the variable flags using
the '@'-character. A range can be decimal (min/max), bitmask or regular
expression (64 byte).
Value ranges for variables can be used to make the environment more
robust against faults such as invalid user input or attacks.
Decimal-range:  foo:da@<min>-<max>
Hexadecimal-range(bitmask): bar:xa@0xdeadbeed
Regex-range: mystring:sa@r"klaus|haus|maus"
Example:
"decimalvalue:dw@100-200,"
    ...
=> env set decimalvalue 1
1 < 100 || 1 > 200
=> env set decimalvalue 150
=>
Signed-off-by: Lukas Funke lukas.funke@weidmueller.com
---
cmd/nvedit.c              |  30 +++---
 env/flags.c               | 217 +++++++++++++++++++++++++++++++++++++-
 include/configs/sandbox.h |   5 +
 include/env_flags.h       |  27 ++++-
 include/search.h          |   1 +
 test/env/Makefile         |   1 +
 test/env/range.c          | 113 ++++++++++++++++++++
 7 files changed, 377 insertions(+), 17 deletions(-)
 create mode 100644 test/env/range.c

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 98a687bcabb..f02545c7a55 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -352,10 +352,13 @@ static int print_static_flags(const char *var_name, const char *flags,
 {
    enum env_flags_vartype type = env_flags_parse_vartype(flags);
    enum env_flags_varaccess access = env_flags_parse_varaccess(flags);
+	struct env_flags_range *range = env_flags_parse_varrange(flags);
-	printf("\t%-20s %-20s %-20s\n", var_name,
+	printf("\t%-20s %-20s %-20s %-20s\n", var_name,
    	env_flags_get_vartype_name(type),
-		env_flags_get_varaccess_name(access));
+		env_flags_get_varaccess_name(access),
+		env_flags_get_varrange_name(range, type));
+	free(range);
return 0;
 }
@@ -364,6 +367,7 @@ static int print_active_flags(struct env_entry *entry)
 {
    enum env_flags_vartype type;
    enum env_flags_varaccess access;
+	struct env_flags_range *range;
if (entry->flags == 0)
    	return 0;
@@ -371,9 +375,11 @@ static int print_active_flags(struct env_entry *entry)
    type = (enum env_flags_vartype)
    	(entry->flags & ENV_FLAGS_VARTYPE_BIN_MASK);
    access = env_flags_parse_varaccess_from_binflags(entry->flags);
-	printf("\t%-20s %-20s %-20s\n", entry->key,
+	range = (struct env_flags_range *)entry->range;
+	printf("\t%-20s %-20s %-20s %-20s\n", entry->key,
    	env_flags_get_vartype_name(type),
-		env_flags_get_varaccess_name(access));
+		env_flags_get_varaccess_name(access),
+		env_flags_get_varrange_name(range, type));
return 0;
 }
@@ -401,19 +407,19 @@ int do_env_flags(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
/* Print the static flags that may exist */
    puts("Static flags:\n");
-	printf("\t%-20s %-20s %-20s\n", "Variable Name", "Variable Type",
-		"Variable Access");
-	printf("\t%-20s %-20s %-20s\n", "-------------", "-------------",
-		"---------------");
+	printf("\t%-20s %-20s %-20s %-20s\n", "Variable Name", "Variable Type",
+		"Variable Access", "Variable Range");
+	printf("\t%-20s %-20s %-20s %-20s\n", "-------------", "-------------",
+		"---------------", "---------------");
    env_attr_walk(ENV_FLAGS_LIST_STATIC, print_static_flags, NULL);
    puts("\n");
/* walk through each variable and print the flags if non-default */
    puts("Active flags:\n");
-	printf("\t%-20s %-20s %-20s\n", "Variable Name", "Variable Type",
-		"Variable Access");
-	printf("\t%-20s %-20s %-20s\n", "-------------", "-------------",
-		"---------------");
+	printf("\t%-20s %-20s %-20s %-20s\n", "Variable Name", "Variable Type",
+		"Variable Access", "Variable Range");
+	printf("\t%-20s %-20s %-20s %-20s\n", "-------------", "-------------",
+		"---------------", "---------------");
    hwalk_r(&env_htab, print_active_flags);
    return 0;
 }
diff --git a/env/flags.c b/env/flags.c
index 233fd460d84..dc0a9c5764f 100644
--- a/env/flags.c
+++ b/env/flags.c
@@ -20,6 +20,9 @@
 #else
 #include <linux/kernel.h>
 #include <env_internal.h>
+#include <vsprintf.h>
+#include <malloc.h>
+#include <slre.h>
 #endif
#ifdef CONFIG_NET
@@ -34,6 +37,8 @@
 #define ENV_FLAGS_WRITEABLE_VARACCESS_REPS ""
 #endif
+#define ENV_FLAGS_RANGE_SEPERATOR '@'
+
 static const char env_flags_vartype_rep[] = "sdxb" ENV_FLAGS_NET_VARTYPE_REPS;
 static const char env_flags_varaccess_rep[] =
    "aroc" ENV_FLAGS_WRITEABLE_VARACCESS_REPS;
@@ -115,6 +120,35 @@ const char *env_flags_get_varaccess_name(enum env_flags_varaccess access)
 {
    return env_flags_varaccess_names[access];
 }
+
+const char *env_flags_get_varrange_name(struct env_flags_range *range,
+					enum env_flags_vartype type)
+{
+	static char range_str[64];
+
+	if (!range)
+		return "";
+
+	switch (type) {
+	case env_flags_vartype_decimal: {
+		snprintf(range_str, sizeof(range_str), "%lld-%lld",
+			 range->u.int_range.min, range->u.int_range.max);
+		break;
+	}
+	case env_flags_vartype_hex: {
+		snprintf(range_str, sizeof(range_str), "0x%llx", range->u.bitmask);
+		break;
+	}
+	case env_flags_vartype_string: {
+		strlcpy(range_str, range->u.re, sizeof(range_str));
+		break;
+	}
+	default:
+		return "";
+	}
+
+	return range_str;
+}
 #endif /* CONFIG_CMD_ENV_FLAGS */
/*
@@ -151,6 +185,9 @@ enum env_flags_varaccess env_flags_parse_varaccess(const char *flags)
    if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC)
    	return va_default;
+	if (flags[ENV_FLAGS_VARACCESS_LOC] == ENV_FLAGS_RANGE_SEPERATOR)
+		return va_default;
+
    access = strchr(env_flags_varaccess_rep,
    	flags[ENV_FLAGS_VARACCESS_LOC]);
@@ -211,6 +248,138 @@ static void skip_num(int hex, const char *value, const char **end,
    	*end = value;
 }
+struct env_flags_range *env_flags_parse_varrange(const char *flags)
+{
+	char *range;
+	struct env_flags_range *r;
+	enum env_flags_vartype type;
+
+	if (!flags)
+		return NULL;
+
+	range = strchr(flags, ENV_FLAGS_RANGE_SEPERATOR);
+	if (!range)
+		return NULL;
+
+	range++;
+
+	r = (struct env_flags_range *)malloc(sizeof(struct env_flags_range));
+	if (!r)
+		return NULL;
+
+	type = env_flags_parse_vartype(flags);
+
+	/* parse regex range r"someregex" */
+	if (!strncmp(range, "r"", 2)) {
+		if (IS_ENABLED(CONFIG_REGEX)) {
+			if (type != env_flags_vartype_string) {
+				free(r);
+				return NULL;
+			}
+
+			char *rstart = strchr(range, '"');
+			char *rend = strrchr(range, '"');
+
+			memset(r->u.re, 0, sizeof(r->u.re));
+
+			if (rstart == rend) /* invalid format r" */
+				return NULL;
+
+			rstart++;
+			if (rstart == rend) /* empty regex is okay */
+				return r;
+
+			if ((rend - rstart) < sizeof(r->u.re))
+				strlcpy(r->u.re, rstart, rend - rstart);
+			else
+				return NULL; /* too big */
+		}
+	} else if (is_hex_prefix(range)) {
+		/* parse bitmask range 0x[a-fA-F0-9]+ */
+		if (type != env_flags_vartype_hex) {
+			free(r);
+			return NULL;
+		}
+		r->u.bitmask = (u64)simple_strtol(range, NULL, 16);
+	} else {
+		/* parse integer range [0-9]+-[0-9]+ */
+		s64 min, max;
+
+		if (type != env_flags_vartype_decimal) {
+			free(r);
+			return NULL;
+		}
+
+		char *lhs = strcpy(r->u.re, range); /* exploit regex buffer */
+		char *rhs = strchr(lhs[0] == '-' ? lhs + 1 : lhs, '-');
+
+		if (!rhs) {
+			free(r);
+			return NULL;
+		}
+
+		*rhs++ = '\0';
+		min = simple_strtol(lhs, NULL, 10);
+		max = simple_strtol(rhs, NULL, 10);
+		r->u.int_range.min = min;
+		r->u.int_range.max = max;
+	}
+
+	return r;
+}
+
+static int _env_flags_validate_range(const struct env_flags_range *range,
+				     enum env_flags_vartype type,
+				     const char *newval)
+{
+	if (!range)
+		return -1;
+
+	switch (type) {
+	case env_flags_vartype_decimal: {
+		s64 value = simple_strtol(newval, NULL, 10);
+		s64 min = range->u.int_range.min;
+		s64 max = range->u.int_range.max;
+
+		if (value < min || value > max) {
+			printf("## Error: value out of bounds\n"
+				"%lld < %lld || %lld > %lld\n", value, min, value, max);
+			return -1;
+		}
+		break;
+	}
+	case env_flags_vartype_hex: {
+		u64 value = (u64)simple_strtoll(newval, NULL, 16);
+		u64 mask = range->u.bitmask;
+
+		if ((value | mask) != mask) {
+			printf("## Error: value out of bounds\n"
+				"value = 0x%llx, mask 0x%llx\n", value, mask);
+			return -1;
+		}
+		break;
+	}
+	case env_flags_vartype_string: {
+#if defined(CONFIG_REGEX)
+		struct slre slre;
+
+		if (slre_compile(&slre, range->u.re)) {
+			if (slre_match(&slre, newval, strlen(newval), NULL) == 0) {
+				printf("## Error: value does not match regex\n"
+				       "value = %s, re = %s\n", newval, range->u.re);
+				return -1;
+			}
+		}
+#endif
+		break;
+	}
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_NET
 int eth_validate_ethaddr_str(const char *addr)
 {
@@ -241,7 +410,7 @@ int eth_validate_ethaddr_str(const char *addr)
  * with that format
  */
 static int _env_flags_validate_type(const char *value,
-	enum env_flags_vartype type)
+				    enum env_flags_vartype type)
 {
    const char *end;
 #ifdef CONFIG_NET
@@ -360,6 +529,20 @@ enum env_flags_varaccess env_flags_get_varaccess(const char *name)
    return env_flags_parse_varaccess(flags);
 }
+/*
+ * Look up the range of a variable directly from the .flags var.
+ */
+struct env_flags_range *env_flags_get_range(const char *name)
+{
+	const char *flags_list = env_get(ENV_FLAGS_VAR);
+	char flags[ENV_FLAGS_ATTR_MAX_LEN + 1];
+
+	if (env_flags_lookup(flags_list, name, flags))
+		return NULL;
+
+	return env_flags_parse_varrange(flags);
+}
+
 /*
  * Validate that the proposed new value for "name" is valid according to the
  * defined flags for that variable, if any.
@@ -395,6 +578,25 @@ int env_flags_validate_varaccess(const char *name, int check_mask)
    return (check_mask & access_mask) != 0;
 }
+/*
+ * Validate that the variable "name" is valid according to
+ * the defined range for that variable, if any.
+ */
+int env_flags_validate_range(const char *name, const char *newval)
+{
+	int r;
+	enum env_flags_vartype type;
+	struct env_flags_range *range;
+
+	type = env_flags_get_type(name);
+	range = env_flags_get_range(name);
+
+	r = _env_flags_validate_range(range, type, newval);
+
+	free(range);
+	return r;
+}
+
 /*
  * Validate the parameters to "env set" directly
  */
@@ -460,8 +662,10 @@ void env_flags_init(struct env_entry *var_entry)
    ret = env_flags_lookup(flags_list, var_name, flags);
/* if any flags were found, set the binary form to the entry */
-	if (!ret && strlen(flags))
+	if (!ret && strlen(flags)) {
    	var_entry->flags = env_parse_flags_to_bin(flags);
+		var_entry->range = env_flags_parse_varrange(flags);
+	}
 }
/*
@@ -489,11 +693,13 @@ static int set_flags(const char *name, const char *value, void *priv)
    /* does the env variable actually exist? */
    if (ep != NULL) {
    	/* the flag list is empty, so clear the flags */
-		if (value == NULL || strlen(value) == 0)
+		if (value == NULL || strlen(value) == 0) {
    		ep->flags = 0;
-		else
+		} else {
    		/* assign the requested flags */
    		ep->flags = env_parse_flags_to_bin(value);
+			ep->range = env_flags_parse_varrange(value);
+		}
    }
return 0;
@@ -547,6 +753,9 @@ int env_flags_validate(const struct env_entry *item, const char *newval,
    			name, newval, env_flags_vartype_rep[type]);
    		return -1;
    	}
+		if (item->range &&
+		    _env_flags_validate_range(item->range, type, newval) < 0)
+			return -1;
    }
/* check for access permission */
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 2372485c84e..8dec2b2cc89 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -21,4 +21,9 @@
 /* Unused but necessary to build */
 #define CFG_SYS_UBOOT_BASE	CONFIG_TEXT_BASE
+#define CFG_ENV_FLAGS_LIST_STATIC \
+	"decimalvalue:da@100-200," \
+	"regexvalue:sa@r"foo|bar"," \
+	"bitmaskvalue:xa@0xf00f"
+
 #endif
diff --git a/include/env_flags.h b/include/env_flags.h
index 2476043b0e3..dfe45ce04d5 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -32,8 +32,19 @@ enum env_flags_varaccess {
    env_flags_varaccess_end
 };
+struct env_flags_range {
+	union  {
+		char re[64];
+		u64 bitmask;
+		struct {
+			s64 min;
+			s64 max;
+		} int_range;
+	} u;
+};
+
 #define ENV_FLAGS_VAR ".flags"
-#define ENV_FLAGS_ATTR_MAX_LEN 2
+#define ENV_FLAGS_ATTR_MAX_LEN 64
 #define ENV_FLAGS_VARTYPE_LOC 0
 #define ENV_FLAGS_VARACCESS_LOC 1
@@ -108,6 +119,11 @@ const char *env_flags_get_vartype_name(enum env_flags_vartype type);
  * Return the name of the access.
  */
 const char *env_flags_get_varaccess_name(enum env_flags_varaccess access);
+/*
+ * Return the range of the access.
+ */
+const char *env_flags_get_varrange_name(struct env_flags_range *range,
+										enum env_flags_vartype type);
 #endif
/*
@@ -118,6 +134,10 @@ enum env_flags_vartype env_flags_parse_vartype(const char *flags);
  * Parse the flags string from a .flags attribute list into the varaccess enum.
  */
 enum env_flags_varaccess env_flags_parse_varaccess(const char *flags);
+/*
+ * Parse the flags string from a .flags attribute list into the varaccess enum.
+ */
+struct env_flags_range *env_flags_parse_varrange(const char *flags);
 /*
  * Parse the binary flags from a hash table entry into the varaccess enum.
  */
@@ -154,6 +174,11 @@ int env_flags_validate_access(const char *name, int check_mask);
  * the defined flags for that variable, if any.
  */
 int env_flags_validate_varaccess(const char *name, int check_mask);
+/*
+ * Validate that the variable "name" is valid according to
+ * the defined range for that variable, if any.
+ */
+int env_flags_validate_range(const char *name, const char *newval)
 /*
  * Validate the parameters passed to "env set" for type compliance
  */
diff --git a/include/search.h b/include/search.h
index 7faf23f4aca..c3de3109a4d 100644
--- a/include/search.h
+++ b/include/search.h
@@ -34,6 +34,7 @@ struct env_entry {
    	int flags);
 #endif
    int flags;
+	void *range;
 };
/*
diff --git a/test/env/Makefile b/test/env/Makefile
index 9a98fd47966..717cbc5555e 100644
--- a/test/env/Makefile
+++ b/test/env/Makefile
@@ -6,3 +6,4 @@ obj-y += cmd_ut_env.o
 obj-y += attr.o
 obj-y += hashtable.o
 obj-$(CONFIG_ENV_IMPORT_FDT) += fdt.o
+obj-y += range.o
diff --git a/test/env/range.c b/test/env/range.c
new file mode 100644
index 00000000000..ed2cf27ef91
--- /dev/null
+++ b/test/env/range.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * (C) Copyright 2024
+ * Lukas Funke, Weidmueller GmbH, lukas.funke@weidmueller.com
+ */
+
+#include <vsprintf.h>
+#include <command.h>
+#include <env_attr.h>
+#include <env_flags.h>
+#include <test/env.h>
+#include <test/ut.h>
+
+/*
+ * Set values according to their range
+ */
+static int env_test_env_set_range(struct unit_test_state *uts)
+{
+	int r;
+	char *value;
+
+	r = env_set("decimalvalue", "100");
+	ut_asserteq(0, r);
+	value = env_get("decimalvalue");
+	ut_asserteq_str("100", value);
+	r = env_set("decimalvalue", "500");
+	ut_asserteq(1, r);
+
+	r = env_set("regexvalue", "foo");
+	ut_asserteq(0, r);
+	value = env_get("regexvalue");
+	ut_asserteq_str("foo", value);
+	r = env_set("regexvalue", "klaus");
+	ut_asserteq(1, r);
+
+	r = env_set("bitmaskvalue", "0xf00f");
+	ut_asserteq(0, r);
+	value = env_get("bitmaskvalue");
+	ut_asserteq_str("0xf00f", value);
+	r = env_set("bitmaskvalue", "0x0110");
+	ut_asserteq(1, r);
+
+	return 0;
+}
+
+ENV_TEST(env_test_env_set_range, 0);
+
+static int env_test_range_to_str(struct unit_test_state *uts)
+{
+	struct env_flags_range range;
+
+	range.u.int_range.min = 123;
+	range.u.int_range.max = 456;
+	ut_asserteq_str("123-456",
+			env_flags_get_varrange_name(&range,
+						    env_flags_vartype_decimal));
+
+	strcpy(&range.u.re[0], "^(some|value)_[0-9a-zA-Z]");
+	ut_asserteq_str("^(some|value)_[0-9a-zA-Z]",
+			env_flags_get_varrange_name(&range,
+						    env_flags_vartype_string));
+
+	range.u.bitmask = 0x12300000;
+	ut_asserteq_str("0x12300000",
+			env_flags_get_varrange_name(&range,
+						    env_flags_vartype_hex));
+	return 0;
+}
+
+ENV_TEST(env_test_range_to_str, 0);
+
+static int env_test_range_parse(struct unit_test_state *uts)
+{
+	char str[64];
+	struct env_flags_range *range;
+
+	range = env_flags_parse_varrange("s@r"somevalue"");
+	ut_assertnonnull(range);
+	free(range);
+
+	range = env_flags_parse_varrange("sw");
+	ut_assertnull(range);
+
+	snprintf(str, sizeof(str), "d@%ld-%ld", LONG_MIN, LONG_MAX);
+	range = env_flags_parse_varrange(str);
+	ut_assertnonnull(range);
+	ut_assert(range->u.int_range.min == LONG_MIN);
+	ut_assert(range->u.int_range.max == LONG_MAX);
+	free(range);
+
+	snprintf(str, sizeof(str), "d@0-%ld", LONG_MAX);
+	range = env_flags_parse_varrange(str);
+	ut_assertnonnull(range);
+	ut_assert(range->u.int_range.min == 0);
+	ut_assert(range->u.int_range.max == LONG_MAX);
+	free(range);
+
+	snprintf(str, sizeof(str), "d@%ld-0", LONG_MIN);
+	range = env_flags_parse_varrange(str);
+	ut_assertnonnull(range);
+	ut_assert(range->u.int_range.min == LONG_MIN);
+	ut_assert(range->u.int_range.max == 0);
+	free(range);
+
+	range = env_flags_parse_varrange("x@0xff0000ff");
+	ut_assertnonnull(range);
+	ut_assert(range->u.bitmask == 0xff0000ff);
+	free(range);
+
+	return 0;
+}
+
+ENV_TEST(env_test_range_parse, 0);
-- 
2.30.2


    

[RFC PATCH 1/1] env: introduce variable ranges

lukas.funke-oss＠weidmueller.com