[U-Boot] [PATCH 1/1] lib/display_options: avoid illegal memory access

* [U-Boot] [PATCH 1/1] lib/display_options: avoid illegal memory access
@ 2019-04-26 16:39 Heinrich Schuchardt
  2019-04-28 21:38 ` Simon Glass
  2019-05-06 11:15 ` Tom Rini
  0 siblings, 2 replies; 3+ messages in thread
From: Heinrich Schuchardt @ 2019-04-26 16:39 UTC (permalink / raw)
  To: u-boot

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>
---
I will take the patch via the u-boot-efi repository.
---
 lib/display_options.c |  4 +++-
 test/print_ut.c       | 20 ++++++++++++--------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/lib/display_options.c b/lib/display_options.c
index af1802ef99..cff20f3755 100644
--- 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 f0f1d6010a..0bc548dca8 100644
--- 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));
--
2.20.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread