lib/display_options: avoid illegal memory access

display_options_get_banner_priv() overwrites bytes before the start of the
buffer if the buffer size is less then 3. This case occurs in the Sandbox
when executing the `ut_print` command.

Correctly handle small buffer sizes. Adjust the print unit test to catch
when bytes before the buffer are overwritten.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>

diff --git a/lib/display_options.c b/lib/display_options.c
index af1802ef992ff7ad8b7083b7bd6cb117d4fd0e1a..cff20f375523be26aceaa88801eaa3723cc21b60 100644 (file)
--- a/lib/display_options.c
+++ b/lib/display_options.c
@@ -23,7 +23,9 @@ char *display_options_get_banner_priv(bool newlines, const char *build_tag,
                                build_tag);
        if (len > size - 3)
                len = size - 3;
-       strcpy(buf + len, "\n\n");
+       if (len < 0)
+               len = 0;
+       snprintf(buf + len, size - len, "\n\n");
 
        return buf;
 }

diff --git a/test/print_ut.c b/test/print_ut.c
index f0f1d6010a164fb2d4e14aa89065ed7c44bdd7f2..0bc548dca831cc09e0de630a7ded77e282a48d1c 100644 (file)
--- a/test/print_ut.c
+++ b/test/print_ut.c
@@ -79,14 +79,18 @@ static int do_ut_print(cmd_tbl_t *cmdtp, int flag, int argc,
        assert(s == str);
        assert(!strcmp("\n\nU-Boo\n\n", s));
 
-       s = display_options_get_banner(true, str, 1);
-       assert(s == str);
-       assert(!strcmp("", s));
-
-       s = display_options_get_banner(true, str, 2);
-       assert(s == str);
-       assert(!strcmp("\n", s));
-
+       /* Assert that we do not overwrite memory before the buffer */
+       str[0] = '`';
+       s = display_options_get_banner(true, str + 1, 1);
+       assert(s == str + 1);
+       assert(!strcmp("`", str));
+
+       str[0] = '~';
+       s = display_options_get_banner(true, str + 1, 2);
+       assert(s == str + 1);
+       assert(!strcmp("~\n", str));
+
+       /* The last two characters are set to \n\n for all buffer sizes > 2 */
        s = display_options_get_banner(false, str, sizeof(str));
        assert(s == str);
        assert(!strcmp("U-Boot \n\n", s));