search

autowp / arduino-mcp2515

Arduino MCP2515 CAN interface library
MIT License
795 stars 279 forks source link

[Security] Out-of-bounds Read & Write with illegal DLC #100

Closed TheAlgorythm closed 1 year ago

TheAlgorythm commented 1 year ago

When the DLC is bigger than 8 (the biggest possible value is 15 on the bit level), the memcpy is out of bounds on both its source and destination.

The relevant parts are: https://github.com/autowp/arduino-mcp2515/blob/master/mcp2515.cpp#L670-L672 https://github.com/autowp/arduino-mcp2515/blob/master/mcp2515.cpp#L610-L612

I would suggest as a minimal mitigation something like that:

dlc = min(dlc, 8);

For reading you could even drop such messages if you want to be strict.

As the DLC is untrusted in readMessage, this is a vulnerability which could be exploited in some applications.

The CVSS could be a 7.8 (AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H/E:F/RL:W/RC:C) (imho conservatively calculated).

TheAlgorythm commented 1 year ago

The problem is that the maximum length of data is 8 which needs 4 bits. But with 4 bits the highest number representable is 15. Therefore an alternative mitigation would be to decrease CAN_MAX_DLEN to 8.

TheAlgorythm commented 1 year ago

Oh, the CAN_MAX_DLEN is already 8. I have looked at it in a copy of this repo where it was changed but I've just looked at the implementation not the definition. Sorry for bothering.