catalinv-ncc opened a new issue, #19170:
URL: https://github.com/apache/nuttx/issues/19170
### Description / Steps to reproduce the issue
drivers/eeprom: Integer Overflow in I2C EEPROM `ee24xx_seek()` Kernel
Operation
## Impact
A compromised user process may use:
* an out-of-bound read to exfiltrate kernel memory, or
* an out-of-bound write to modify the kernel memory.
At a minimum, accessing out-of-bounds memory cause a device crash.
## Description
When accessing the I2C EEPROM storage, the user applications will use code
such as
```c
fd = open("/dev/eeprom");
num_bytes_read = read(fd, buffer, len);
```
These calls will use the following kernel driver code:
```c
static const struct file_operations g_ee24xx_fops =
{
ee24xx_open, /* open */
ee24xx_close, /* close */
ee24xx_read, /* read */
ee24xx_write, /* write */
ee24xx_seek, /* seek */
ee24xx_ioctl, /* ioctl */
};
int ee24xx_initialize(FAR struct i2c_master_s *bus, uint8_t devaddr,
FAR char *devname, int devtype, int readonly)
{
...
return register_driver_with_size(devname, &g_ee24xx_fops, 0666, eedev,
eedev->size);
}
```
The function seek which allows the user to move the cursor to a particular
offset in order to read and write from EEPROM storage does not validate the
offset is valid. Later, this can cause an out-of-bounds reads or writes. Note
that `newpos` may store a large value, larger than the size of the EEPROM.
```c
static off_t ee24xx_seek(FAR struct file *filep, off_t offset, int whence)
{
...
ret = nxmutex_lock(&eedev->lock);
if (ret < 0)
{
return ret;
}
switch (whence)
{
case SEEK_CUR:
newpos = filep->f_pos + offset;
break;
case SEEK_SET:
newpos = offset;
break;
case SEEK_END:
newpos = eedev->size + offset;
break;
default:
/* Return EINVAL if the whence argument is invalid */
nxmutex_unlock(&eedev->lock);
return -EINVAL;
}
if (newpos >= 0)
{
filep->f_pos = newpos;
ret = newpos;
finfo("SEEK newpos %" PRIdOFF "\n", newpos);
}
else
{
ret = -EINVAL;
}
...
}
```
When the I2C EEPROM kernel driver is initialized, the size (in bytes) of the
storage device is stored in `eedev->size`.
Later in `ee24xx_read()` and `ee24xx_write()` can be tricked and bypass the
checks below. For the read operation, the cursor can be set to a very large
value, and `filep->f_pos + len` can overflow, resulting in a small value, less
than `eedev->size`. For the write operation, the cursor needs to be in the
normal range, i.e. `0` to `eedev->size` due to the first check, but if the
value of `len` is very large `len + filep->f_pos` will overflow and will bypass
the second check.
```c
static ssize_t ee24xx_read(FAR struct file *filep, FAR char *buffer, size_t
len)
{
...
/* trim len if read would go beyond end of device */
if ((filep->f_pos + len) > eedev->size)
{
len = eedev->size - filep->f_pos;
}
...
}
static ssize_t ee24xx_write(FAR struct file *filep, FAR const char *buffer,
size_t len)
{
...
if (filep->f_pos >= eedev->size)
{
return -EFBIG;
}
...
/* Clamp len to avoid crossing the end of the memory */
if ((len + filep->f_pos) > eedev->size)
{
len = eedev->size - filep->f_pos;
finfo("Len clamped to %zu\n", len);
}
```
## Recommendation
Check that `filep->f_pos + len` operations performed by the read and write
functions do not overflow, otherwise return an error code.
### On which OS does this issue occur?
[OS: Linux]
### What is the version of your OS?
Ubuntu 24.04
### NuttX Version
master
### Issue Architecture
[Arch: all]
### Issue Area
[Area: Drivers]
### Host information
_No response_
### Verification
- [x] I have verified before submitting the report.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
