Off-By-One Error in i2c_writeBuffer about i2c-exp-driver HOT 4 CLOSED

(thanks for pointing me to this from the Omega forums)

good catch on this increasing buffer size, but, i think it doesn't affect the intended functionality once in _i2c_writeBuffer. So, inside of i2c_writeBuffer, in your example, size was: 3 | is: 4 and bufferNew is 5. Once passed to _i2c_writeBuffer, the actual write command still uses size (which is 4), so you'll be writing the address + 3 bytes. Only consequence I see, is that you're wasting the time and space to store garbage in bufferNew[5] (or am I wrong?)

// generic function to write a buffer to the i2c bus
int _i2c_writeBuffer(int devNum, int devAddr, uint8_t *buffer, int size)
{
	int 	status;
	int 	fd, index;
        ......
	// write to the i2c device
	status = write(fd, buffer, size);
	..........
	return (status);
}`

from i2c-exp-driver.

It will correctly write the write number of bytes to the I2C device (address + initially given buffer), right. (However bufferNew has 4 byte allocated, not 5, and the overwritten index is bufferNew[4], the 5th byte from the start, but the rest is correct)

However the damage is already done after the memcpy beyond the boundaries of bufferNew because the allocated / malloc()ed space just isn't big enough (by 1 byte). This will cause an overwrite into some other memory structure on the heap or a heap information block, which can cause subsequent program failure.

Memory / heap corruption might even be exploitable in special settings, i.e. be used for a system takeover by attacking the programs using this I2C library (https://googleprojectzero.blogspot.de/2015/06/what-is-good-memory-corruption.html).

from i2c-exp-driver.

@maxgerhardt yep, you were absolutely correct, we were copying by the incremented size instead of the actual size.
We've released v0.7 of libonioni2c, please try it out and let us know if there's any issues.

from i2c-exp-driver.

Yep, this off-by-one was fixed.

from i2c-exp-driver.