Re: [BUG] Stack buffer overflow WRITE of size 1 in barebox_printf function

[Sascha Hauer <sha@pengutronix.de>]

Hi,

On Sun, Apr 18, 2021 at 12:49:16AM +0530, Neeraj Pal wrote:
> Hi,
> 
> I have found the stack buffer overflow issue with WRITE of size 1 in
> barebox_printf function common/console_common.c:240 which further goes
> and crashes into a call vsnprintf lib/vsprintf.c:440
> 
> Tested on:
> - barebox-2021.04.0
> - git commit af0f068a6edad45b033e772056ac0352e1ba3613

Thanks again for reporting. I can confirm this issue happens here as well.

It happens because we are printing into fixed size buffers without
checking the length. The following changes this to use (v)snprintf
instead and should fix this issue.

Regards,
 Sascha

-------------------------------8<----------------------------------

>From a4221fe41b8d4a4b49f533e2869719b721416ff4 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Fri, 7 May 2021 11:37:27 +0200
Subject: [PATCH] console: Fix printbuffer overflowing

The barebox printf functions are not safe against too long strings. The
pattern is always the same: We (v)sprintf into a fixed size buffer. Use
(v)snprintf instead to not overwrite the fixed size buffer. We stand
back from using dynamically sized buffer though, as the barebox printf
like functions might be called before the malloc pool is initialzed.

Reported-by: Neeraj Pal <neerajpal09@gmail.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/console_common.c | 14 +++++++-------
 pbl/console.c           |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/common/console_common.c b/common/console_common.c
index 4c1230464c..2460fb21bd 100644
--- a/common/console_common.c
+++ b/common/console_common.c
@@ -126,7 +126,7 @@ int pr_print(int level, const char *fmt, ...)
 		return 0;
 
 	va_start(args, fmt);
-	i = vsprintf(printbuffer, fmt, args);
+	i = vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 	va_end(args);
 
 	pr_puts(level, printbuffer);
@@ -144,13 +144,13 @@ int dev_printf(int level, const struct device_d *dev, const char *format, ...)
 		return 0;
 
 	if (dev->driver && dev->driver->name)
-		ret += sprintf(printbuffer, "%s ", dev->driver->name);
+		ret += snprintf(printbuffer, CFG_PBSIZE - ret, "%s ", dev->driver->name);
 
-	ret += sprintf(printbuffer + ret, "%s: ", dev_name(dev));
+	ret += snprintf(printbuffer + ret, CFG_PBSIZE - ret, "%s: ", dev_name(dev));
 
 	va_start(args, format);
 
-	ret += vsprintf(printbuffer + ret, format, args);
+	ret += vsnprintf(printbuffer + ret, CFG_PBSIZE - ret, format, args);
 
 	va_end(args);
 
@@ -235,7 +235,7 @@ int printf(const char *fmt, ...)
 	 * For this to work, printbuffer must be larger than
 	 * anything we ever want to print.
 	 */
-	i = vsprintf (printbuffer, fmt, args);
+	i = vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 	va_end(args);
 
 	/* Print the string */
@@ -254,7 +254,7 @@ int vprintf(const char *fmt, va_list args)
 	 * For this to work, printbuffer must be larger than
 	 * anything we ever want to print.
 	 */
-	i = vsprintf(printbuffer, fmt, args);
+	i = vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 
 	/* Print the string */
 	puts(printbuffer);
@@ -342,7 +342,7 @@ int dprintf(int file, const char *fmt, ...)
 	 * For this to work, printbuffer must be larger than
 	 * anything we ever want to print.
 	 */
-	vsprintf(printbuffer, fmt, args);
+	vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 	va_end(args);
 
 	/* Print the string */
diff --git a/pbl/console.c b/pbl/console.c
index 007e4e4b83..ec96b20054 100644
--- a/pbl/console.c
+++ b/pbl/console.c
@@ -54,7 +54,7 @@ int printf(const char *fmt, ...)
 	char printbuffer[CFG_PBSIZE];
 
 	va_start(args, fmt);
-	i = vsprintf(printbuffer, fmt, args);
+	i = vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 	va_end(args);
 
 	console_puts(CONSOLE_STDOUT, printbuffer);
@@ -69,7 +69,7 @@ int pr_print(int level, const char *fmt, ...)
 	char printbuffer[CFG_PBSIZE];
 
 	va_start(args, fmt);
-	i = vsprintf(printbuffer, fmt, args);
+	i = vsnprintf(printbuffer, CFG_PBSIZE, fmt, args);
 	va_end(args);
 
 	console_puts(CONSOLE_STDOUT, printbuffer);
-- 
2.29.2



-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox