From 8f8c04bf1ebbd2f72f1643e7ad9617dafa6e5409 Mon Sep 17 00:00:00 2001 From: Nicolas Iooss Date: Fri, 10 Jun 2022 14:50:25 +0000 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 Reviewed-by: Heiko Schocher --- cmd/i2c.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/i2c.c b/cmd/i2c.c index 9050b2b8d27a..bd04b14024be 100644 --- a/cmd/i2c.c +++ b/cmd/i2c.c @@ -200,10 +200,10 @@ void i2c_init_board(void) * * 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++) { @@ -247,7 +247,7 @@ static int do_i2c_read(struct cmd_tbl *cmdtp, int flag, int argc, { uint chip; uint devaddr, length; - int alen; + uint alen; u_char *memaddr; int ret; #if CONFIG_IS_ENABLED(DM_I2C) @@ -301,7 +301,7 @@ static int do_i2c_write(struct cmd_tbl *cmdtp, int flag, int argc, { uint chip; uint devaddr, length; - int alen; + uint alen; u_char *memaddr; int ret; #if CONFIG_IS_ENABLED(DM_I2C) @@ -469,8 +469,8 @@ static int do_i2c_md(struct cmd_tbl *cmdtp, int flag, int argc, { uint chip; uint addr, length; - int alen; - int j, nbytes, linebytes; + uint alen; + uint j, nbytes, linebytes; int ret; #if CONFIG_IS_ENABLED(DM_I2C) struct udevice *dev; @@ -589,9 +589,9 @@ static int do_i2c_mw(struct cmd_tbl *cmdtp, int flag, int argc, { uint chip; ulong addr; - int alen; + uint alen; uchar byte; - int count; + uint count; int ret; #if CONFIG_IS_ENABLED(DM_I2C) struct udevice *dev; @@ -676,8 +676,8 @@ static int do_i2c_crc(struct cmd_tbl *cmdtp, int flag, int argc, { uint chip; ulong addr; - int alen; - int count; + uint alen; + uint count; uchar byte; ulong crc; ulong err; @@ -985,7 +985,7 @@ static int do_i2c_loop(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { uint chip; - int alen; + uint alen; uint addr; uint length; u_char bytes[16];