[PATCH] i2c: fix stack buffer overflow vulnerability in i2c md command

[Tom Rini]

On Fri, Jun 10, 2022 at 02:50:25PM +0000, nicolas.iooss.ledger at proton.me wrote:

> From: Nicolas Iooss <nicolas.iooss+uboot at ledger.fr>
> 
> 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 at ledger.fr>
> Reviewed-by: Heiko Schocher <hs at denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20220628/f3ad3bd9/attachment.sig>