diff mbox series

[dunfell,PATCHv2] u-boot: fix CVE-2022-34835

Message ID 20220830190039.48510-1-flowergom@gmail.com
State New, archived
Headers show
Series [dunfell,PATCHv2] u-boot: fix CVE-2022-34835 | expand

Commit Message

Minjae Kim Aug. 30, 2022, 7 p.m. UTC
i2c: fix stack buffer overflow vulnerability in i2c md command

CVE: CVE-2022-34835

Signed-off-by:Minjae Kim <flowergom@gmail.com>
---
 .../u-boot/files/CVE-2022-34835.patch         | 124 ++++++++++++++++++
 meta/recipes-bsp/u-boot/u-boot_2020.01.bb     |   4 +
 2 files changed, 128 insertions(+)
 create mode 100644 meta/recipes-bsp/u-boot/files/CVE-2022-34835.patch

Comments

Tom Rini Aug. 31, 2022, 3:05 p.m. UTC | #1
On Tue, Aug 30, 2022 at 09:00:39PM +0200, Minjae Kim wrote:

> i2c: fix stack buffer overflow vulnerability in i2c md command
> 
> CVE: CVE-2022-34835
> 
> Signed-off-by:Minjae Kim <flowergom@gmail.com>

While this is the full backport of the fix for the issue, at this point
we now also have:
https://patchwork.ozlabs.org/project/uboot/list/?series=315609&state=*
because the initial fix broke other real uses of the command.
diff mbox series

Patch

diff --git a/meta/recipes-bsp/u-boot/files/CVE-2022-34835.patch b/meta/recipes-bsp/u-boot/files/CVE-2022-34835.patch
new file mode 100644
index 0000000000..9d69828c98
--- /dev/null
+++ b/meta/recipes-bsp/u-boot/files/CVE-2022-34835.patch
@@ -0,0 +1,124 @@ 
+From 26cb16c9d8b5ee3730474ae67ebd14eb4b30e0c6 Mon Sep 17 00:00:00 2001
+From: Nicolas Iooss <nicolas.iooss+uboot@ledger.fr>
+Date: Tue, 30 Aug 2022 20:48:54 +0200
+Subject: [PATCH] i2c: fix stack buffer overflow vulnerability in i2c md
+ command
+
+When running "i2c md 0 0 80000100", the function do_i2c_md parses the
+length into an unsigned int variable named length. The value is then
+moved to a signed variable:
+
+    int nbytes = length;
+    #define DISP_LINE_LEN 16
+    int linebytes = (nbytes > DISP_LINE_LEN) ? DISP_LINE_LEN : nbytes;
+    ret = dm_i2c_read(dev, addr, linebuf, linebytes);
+
+On systems where integers are 32 bits wide, 0x80000100 is a negative
+value to "nbytes > DISP_LINE_LEN" is false and linebytes gets assigned
+0x80000100 instead of 16.
+
+The consequence is that the function which reads from the i2c device
+(dm_i2c_read or i2c_read) is called with a 16-byte stack buffer to fill
+but with a size parameter which is too large. In some cases, this could
+trigger a crash. But with some i2c drivers, such as drivers/i2c/nx_i2c.c
+(used with "nexell,s5pxx18-i2c" bus), the size is actually truncated to
+a 16-bit integer. This is because function i2c_transfer expects an
+unsigned short length. In such a case, an attacker who can control the
+response of an i2c device can overwrite the return address of a function
+and execute arbitrary code through Return-Oriented Programming.
+
+Fix this issue by using unsigned integers types in do_i2c_md. While at
+it, make also alen unsigned, as signed sizes can cause vulnerabilities
+when people forgot to check that they can be negative.
+
+Signed-off-by: Nicolas Iooss <nicolas.iooss+uboot@ledger.fr>
+Reviewed-by: Heiko Schocher <hs@denx.de>
+
+Upstream-Status: Backport [https://github.com/u-boot/u-boot/commit/8f8c04bf1ebbd2f72f1643e7ad9617dafa6e5409]
+Signed-off-by:Minjae Kim <flowergom@gmail.com>
+---
+ cmd/i2c.c | 24 ++++++++++++------------
+ 1 file changed, 12 insertions(+), 12 deletions(-)
+
+diff --git a/cmd/i2c.c b/cmd/i2c.c
+index 43a76299b3..c54b88a1d8 100644
+--- a/cmd/i2c.c
++++ b/cmd/i2c.c
+@@ -246,10 +246,10 @@ int i2c_set_bus_speed(unsigned int speed)
+  *
+  * Returns the address length.
+  */
+-static uint get_alen(char *arg, int default_len)
++static uint get_alen(char *arg, uint default_len)
+ {
+-	int	j;
+-	int	alen;
++	uint	j;
++	uint	alen;
+ 
+ 	alen = default_len;
+ 	for (j = 0; j < 8; j++) {
+@@ -292,7 +292,7 @@ static int do_i2c_read ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
+ {
+ 	uint	chip;
+ 	uint	devaddr, length;
+-	int alen;
++	uint alen;
+ 	u_char  *memaddr;
+ 	int ret;
+ #ifdef CONFIG_DM_I2C
+@@ -345,7 +345,7 @@ static int do_i2c_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[
+ {
+ 	uint	chip;
+ 	uint	devaddr, length;
+-	int alen;
++	uint alen;
+ 	u_char  *memaddr;
+ 	int ret;
+ #ifdef CONFIG_DM_I2C
+@@ -511,8 +511,8 @@ static int do_i2c_md ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
+ {
+ 	uint	chip;
+ 	uint	addr, length;
+-	int alen;
+-	int	j, nbytes, linebytes;
++	uint alen;
++	uint	j, nbytes, linebytes;
+ 	int ret;
+ #ifdef CONFIG_DM_I2C
+ 	struct udevice *dev;
+@@ -630,9 +630,9 @@ static int do_i2c_mw ( cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
+ {
+ 	uint	chip;
+ 	ulong	addr;
+-	int	alen;
++	uint	alen;
+ 	uchar	byte;
+-	int	count;
++	uint	count;
+ 	int ret;
+ #ifdef CONFIG_DM_I2C
+ 	struct udevice *dev;
+@@ -716,8 +716,8 @@ static int do_i2c_crc (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
+ {
+ 	uint	chip;
+ 	ulong	addr;
+-	int	alen;
+-	int	count;
++	uint	alen;
++	uint	count;
+ 	uchar	byte;
+ 	ulong	crc;
+ 	ulong	err;
+@@ -1023,7 +1023,7 @@ static int do_i2c_probe (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv
+ static int do_i2c_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+ {
+ 	uint	chip;
+-	int alen;
++	uint    alen;
+ 	uint	addr;
+ 	uint	length;
+ 	u_char	bytes[16];
+-- 
+2.25.1
+
diff --git a/meta/recipes-bsp/u-boot/u-boot_2020.01.bb b/meta/recipes-bsp/u-boot/u-boot_2020.01.bb
index 02d67c0db2..16e2340bb6 100644
--- a/meta/recipes-bsp/u-boot/u-boot_2020.01.bb
+++ b/meta/recipes-bsp/u-boot/u-boot_2020.01.bb
@@ -2,3 +2,7 @@  require u-boot-common.inc
 require u-boot.inc
 
 DEPENDS += "bc-native dtc-native"
+
+SRC_URI_append = " \
+    file://CVE-2022-34835.patch \
+"