On Mon, Jun 27, 2022 at 06:33:01AM +0200, Heiko Schocher wrote:
Hello Nicolas,
On 21.06.22 16:04, Nicolas IOOSS wrote:
Hello,
I sent some days ago the vulnerability fix below. I have not received any reply yet. Could a maintainer take a look at it, please?
Sorry for that, but I was on the road (embedded world in nuremberg).
Best regards, Nicolas
------- Original Message ------- Le vendredi 10 juin 2022 à 4:50 PM, nicolas.iooss.ledger@proton.me a écrit :
From: Nicolas Iooss nicolas.iooss+uboot@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@ledger.fr
cmd/i2c.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
Reviewed-by: Heiko Schocher hs@denx.de
@Tom: Should we add this to 2022.07? If so, feel free to pick it up, thanks!
I'll pick this up directly along with the squashfs fix in the next day or two, thanks.