The names of the "BOOT_<slot>_LEFT" environment variables are built with
sprintf() into fixed 42-byte stack buffers, and the "rauc.slot=<slot>"
kernel argument into a 64-byte one. The slot names come from the
BOOT_ORDER environment variable in find_active_slot() and
distro_rauc_boot(), and from CONFIG_BOOTMETH_RAUC_BOOT_ORDER in
distro_rauc_read_bootflow() and the reset-all-tries path. Nothing limits
their length, so a slot name longer than 32 characters writes past the
end of the buffer, and even a 32-character one overflows it by the NUL
terminator.

The BOOT_ORDER entries are checked against the configured slots when the
bootflow is scanned, but the variable is read again when the bootflow is
booted, so changing it in between (e.g. from the command line) reaches
the unchecked sprintf() calls.

Build the variable names through a small helper that uses snprintf() and
rejects slot names which do not fit, size the buffer for a 32-character
slot name plus NUL, and use snprintf() for the kernel argument as well.
Its slot name has passed the same check by then, so it always fits.

Fixes: 7e5c2c782fb9 ("bootstd: Add implementation for bootmeth rauc")
Signed-off-by: Aristo Chen <[email protected]>
---
 boot/bootmeth_rauc.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/boot/bootmeth_rauc.c b/boot/bootmeth_rauc.c
index 844df7576bb..123ec913cb5 100644
--- a/boot/bootmeth_rauc.c
+++ b/boot/bootmeth_rauc.c
@@ -22,8 +22,8 @@
 #include <linux/stringify.h>
 #include <asm/cache.h>
 
-/* Length of env var "BOOT_*_LEFT" */
-#define BOOT_LEFT_LEN  (5 + 32 + 5)
+/* Size of the name of env var "BOOT_<slot>_LEFT", for a 32-char slot name */
+#define BOOT_LEFT_LEN  (5 + 32 + 5 + 1)
 
 static const char * const script_names[] = { "boot.scr", "boot.scr.uimg", NULL 
};
 
@@ -79,6 +79,21 @@ static struct distro_rauc_slot *get_slot(struct 
distro_rauc_priv *priv,
        return NULL;
 }
 
+/**
+ * boot_left_name() - Build the name of the "BOOT_<slot>_LEFT" env var
+ *
+ * @buf: Buffer of BOOT_LEFT_LEN bytes to write the name to
+ * @slot: Name of the slot
+ * Return: 0 if OK, -ENAMETOOLONG if @slot does not fit
+ */
+static int boot_left_name(char *buf, const char *slot)
+{
+       if (snprintf(buf, BOOT_LEFT_LEN, "BOOT_%s_LEFT", slot) >= BOOT_LEFT_LEN)
+               return -ENAMETOOLONG;
+
+       return 0;
+}
+
 static int distro_rauc_check(struct udevice *dev, struct bootflow_iter *iter)
 {
        /*
@@ -167,7 +182,11 @@ static int distro_rauc_read_bootflow(struct udevice *dev, 
struct bootflow *bflow
        default_boot_order = CONFIG_BOOTMETH_RAUC_BOOT_ORDER;
        default_boot_order_list = str_to_list(default_boot_order);
        for (i = 0; default_boot_order_list[i]; i++) {
-               sprintf(boot_left, "BOOT_%s_LEFT", default_boot_order_list[i]);
+               ret = boot_left_name(boot_left, default_boot_order_list[i]);
+               if (ret) {
+                       str_free_list(default_boot_order_list);
+                       return log_msg_ret("name", ret);
+               }
                if (!env_get(boot_left)) {
                        log_debug("%s did not exist yet, setting default 
value\n",
                                  boot_left);
@@ -333,7 +352,11 @@ static int find_active_slot(char **slot_name, ulong 
*slot_tries)
                return log_msg_ret("env", -ENOENT);
        boot_order_list = str_to_list(boot_order);
        for (i = 0; boot_order_list[i] && !slot_found; i++) {
-               sprintf(boot_left, "BOOT_%s_LEFT", boot_order_list[i]);
+               ret = boot_left_name(boot_left, boot_order_list[i]);
+               if (ret) {
+                       str_free_list(boot_order_list);
+                       return log_msg_ret("name", ret);
+               }
                tries = env_get_ulong(boot_left, 10, ULONG_MAX);
                if (tries == ULONG_MAX) {
                        str_free_list(boot_order_list);
@@ -359,7 +382,11 @@ static int find_active_slot(char **slot_name, ulong 
*slot_tries)
                        log_info("INFO: Resetting all slot tries to " 
__stringify(CONFIG_BOOTMETH_RAUC_DEFAULT_TRIES) "\n");
                        boot_order_list = 
str_to_list(CONFIG_BOOTMETH_RAUC_BOOT_ORDER);
                        for (i = 0; boot_order_list[i]; i++) {
-                               sprintf(boot_left, "BOOT_%s_LEFT", 
boot_order_list[i]);
+                               ret = boot_left_name(boot_left, 
boot_order_list[i]);
+                               if (ret) {
+                                       str_free_list(boot_order_list);
+                                       return log_msg_ret("name", ret);
+                               }
                                ret = env_set_ulong(boot_left, 
CONFIG_BOOTMETH_RAUC_DEFAULT_TRIES);
                                if (ret) {
                                        str_free_list(boot_order_list);
@@ -416,7 +443,7 @@ static int distro_rauc_boot(struct udevice *dev, struct 
bootflow *bflow)
                return log_msg_ret("env", ret);
 
        /* Kernel command line arguments */
-       sprintf(raucargs, "rauc.slot=%s", active_slot);
+       snprintf(raucargs, sizeof(raucargs), "rauc.slot=%s", active_slot);
        ret = env_set("raucargs", raucargs);
        if (ret)
                return log_msg_ret("env", ret);
@@ -454,7 +481,8 @@ static int distro_rauc_boot(struct udevice *dev, struct 
bootflow *bflow)
        log_debug("BOOT_ORDER: %s\n", boot_order);
        boot_order_list = str_to_list(boot_order);
        for (i = 0; boot_order_list[i]; i++) {
-               sprintf(boot_left, "BOOT_%s_LEFT", boot_order_list[i]);
+               if (boot_left_name(boot_left, boot_order_list[i]))
+                       continue;
                log_debug("%s: %s\n", boot_left, env_get(boot_left));
        }
        str_free_list(boot_order_list);
-- 
2.43.0

Reply via email to