i2cdevlib icon indicating copy to clipboard operation
i2cdevlib copied to clipboard

Count overflow in Arduino/I2Cdev::readBytes and I2Cdev::readWords

Open nekomona opened this issue 1 year ago • 1 comments

The data type of count in these two functions are incorrectly being int8_t, while length is uint8_t. This will cause an overflow when transmitting data with length > 128 and corrupt the data before buffer.

https://github.com/jrowberg/i2cdevlib/blob/2a0d98f709bbf3a3c052aa8d73f74d30295e5689/Arduino/I2Cdev/I2Cdev.cpp#L208-L222

More occurrences have been found in #750 .

Below is an overflow captured when reading 168 bytes from a MPU6050 FIFO, which caused function frame corruption and crashed the program. image

nekomona avatar May 28 '23 13:05 nekomona

It seems that the sign of count is used to report failure, which need to be taken into consideration: Whether should it limit length within 127 bytes, using 0~254 as normal count and 255 as failure, or extend the data width.

Currently it's the second one with implicit type conversion. Code detecting failure by comparing with -1 still works, yet if it only compares with 0, reads with length > 127 will be misinterpreted as failed even if the result is correct.

nekomona avatar May 28 '23 13:05 nekomona