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

[Heiko Schocher]

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 at proton.me> a écrit :
> 
> 
>> 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
>>
>> ---
>> cmd/i2c.c | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Heiko Schocher <hs at denx.de>

@Tom: Should we add this to 2022.07? If so, feel free to pick it up,
      thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de