env: Fix possible out-of-bound access in env_do_env_set

It was discovered that env_do_env_set() currently suffer from a long
time of a possible out-of-bound access for the argv array handling.

The BUG is present in the function env_do_env_set() line:

name = argv[1];

where the function at this point assume the argv at index 1 is always
present and can't be NULL. Aside from the fact that it's always
better to validate argv entry with the argc variable, situation where
the argv[1] is NULL is actually possible and not an error condition.

A example of where an out-of-bound access is triggered is with the
command "askenv - Press ENTER to ...".
This is a common pattern for bootmenu entry to ask the user input after
a bootmenu command succeeded.

In the context of such command, the while loop before "name = argv[1];"
parse the "-" char as an option arg and increment the argv pointer by
one (to make the rest of the logic code ignore the option argv) and
decrement argc value.

The while loop logic is correct but at the "name = argv[1];" line, the
argv have only one element left (the "-" char) and accessing argv[1]
(aka the secong element from argv pointer) cause an out-of-bound access
(making the bootloader eventually crash with strchr searching in invalid
data)

To better handle this and prevent the out-of-bound access, actually
check the argv entry left (with the use of the argc variable) and exit
early before doing any kind of array access.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

diff --git a/env/common.c b/env/common.c
index 1e23c5de436f87ef30bb8c2a5d2cbe417bf2f26a..05e78d6387430745e20319f5806133917aa5efd1 100644 (file)
--- a/env/common.c
+++ b/env/common.c
@@ -82,6 +82,10 @@ int env_do_env_set(int flag, int argc, char *const argv[], int env_flag)
                }
        }
        debug("Final value for argc=%d\n", argc);
+       /* Exit early if we don't have an env to apply */
+       if (argc < 2)
+               return 0;
+
        name = argv[1];
 
        if (strchr(name, '=')) {