board: nanopi2: fix bd_update_env() cmdline buffer overflow

Replace unbounded strcpy()/sprintf() calls with snprintf() and
check the return value against remaining buffer capacity at each
append step. The previous size guard did not account for
subsequent dpi suffix, remaining bootargs tail, and bootdev
token appends, allowing overflow when those later writes exceed
the remaining space.

Fixes: d1611086e005 ("arm: add support for SoC s5p4418 (cpu) / nanopi2 board")
To: u-boot@lists.denx.de

Signed-off-by: Ngo Luong Thanh Tra <S4210155@student.rmit.edu.au>
Tested-by: Stefan Bosch <stefan_b@posteo.net>

diff --git a/board/friendlyarm/nanopi2/board.c b/board/friendlyarm/nanopi2/board.c
index 4dff32e10d6b7c0582ee0a0496b5b806b7e5f2c1..eb10cd5143d1fefa8b35c1f655d73187161052ca 100644 (file)
--- a/board/friendlyarm/nanopi2/board.c
+++ b/board/friendlyarm/nanopi2/board.c
@@ -328,6 +328,7 @@ static void bd_update_env(void)
 #define CMDLINE_LCD            " lcd="
        char cmdline[CONFIG_SYS_CBSIZE];
        int n = 1;
+       int ret;
 
        if (rootdev != CONFIG_ROOT_DEV && !env_get("firstboot")) {
                env_set_ulong("rootdev", rootdev);
@@ -347,49 +348,63 @@ static void bd_update_env(void)
        else
                cmdline[0] = '\0';
 
-       if ((n + strlen(name) + sizeof(CMDLINE_LCD)) > sizeof(cmdline)) {
-               printf("Error: `bootargs' is too large (%d)\n", n);
-               goto __exit;
-       }
-
        if (bootargs) {
                p = strstr(bootargs, CMDLINE_LCD);
                if (p) {
                        n = (p - bootargs);
                        p += strlen(CMDLINE_LCD);
                }
-               strncpy(cmdline, bootargs, n);
+               ret = snprintf(cmdline, sizeof(cmdline), "%.*s", n, bootargs);
+               if (ret < 0 || ret >= (int)sizeof(cmdline))
+                       goto __exit;
        }
 
        /* add `lcd=NAME,NUMdpi' */
-       strncpy(cmdline + n, CMDLINE_LCD, strlen(CMDLINE_LCD));
-       n += strlen(CMDLINE_LCD);
+       ret = snprintf(cmdline + n, sizeof(cmdline) - n, "%s", CMDLINE_LCD);
+       if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+               goto __exit;
+       n += ret;
 
-       strcpy(cmdline + n, name);
-       n += strlen(name);
+       ret = snprintf(cmdline + n, sizeof(cmdline) - n, "%s", name);
+       if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+               goto __exit;
+       n += ret;
 
        if (lcddpi) {
-               n += sprintf(cmdline + n, ",%sdpi", lcddpi);
+               ret = snprintf(cmdline + n, sizeof(cmdline) - n, ",%sdpi", lcddpi);
+               if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+                       goto __exit;
+               n += ret;
        } else {
                int dpi = bd_get_lcd_density();
 
-               if (dpi > 0 && dpi < 600)
-                       n += sprintf(cmdline + n, ",%ddpi", dpi);
+               if (dpi > 0 && dpi < 600) {
+                       ret = snprintf(cmdline + n, sizeof(cmdline) - n, ",%ddpi", dpi);
+                       if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+                               goto __exit;
+                       n += ret;
+               }
        }
 
        /* copy remaining of bootargs */
        if (p) {
                p = strstr(p, " ");
                if (p) {
-                       strcpy(cmdline + n, p);
-                       n += strlen(p);
+                       ret = snprintf(cmdline + n, sizeof(cmdline) - n, "%s", p);
+                       if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+                               goto __exit;
+                       n += ret;
                }
        }
 
        /* append `bootdev=2' */
 #define CMDLINE_BDEV   " bootdev="
-       if (rootdev > 0 && !strstr(cmdline, CMDLINE_BDEV))
-               n += sprintf(cmdline + n, "%s2", CMDLINE_BDEV);
+       if (rootdev > 0 && !strstr(cmdline, CMDLINE_BDEV)) {
+               ret = snprintf(cmdline + n, sizeof(cmdline) - n, "%s2", CMDLINE_BDEV);
+               if (ret < 0 || ret >= (int)(sizeof(cmdline) - n))
+                       goto __exit;
+               n += ret;
+       }
 
        /* finally, let's update uboot env & save it */
        if (bootargs && strncmp(cmdline, bootargs, sizeof(cmdline))) {