K-ANOY opened a new issue, #21245:
URL: https://github.com/apache/nuttx/issues/21245
### Description / Steps to reproduce the issue
In `drivers/input/st7123.c` (ST7123 I2C touch controller) the
device-reported `max_touches` value
sizes a single I2C burst read into the fixed 74-byte frame buffer with no
upper clamp — the promised
clamp exists only in a comment, so the bound is **absent** in all build
configurations. This is a
heap out-of-bounds **write** (CWE-787) of up to 1715 bytes of
attacker-controlled I2C data past the
74-byte frame field, clobbering the rest of the device structure and
adjacent heap (High severity).
The attacker is a malicious/compromised ST7123 touch controller on the I2C
bus.
At probe, the driver reads the controller's capability block (including the
`uint8_t`
`max_touches` at register `0x09`) over I2C, then uses that value — with no
upper clamp — to size
a single I2C burst read into the fixed-size frame buffer on every touch
interrupt:
```c
/* drivers/input/st7123.c */
830 st7123_read_sequential(dev, ST7123_TOUCH_MAX_X_COORD_H,
(uint8_t *) &dev->device_caps,
sizeof(dev->device_caps));
...
955 if (priv->device_caps.max_touches == 0) /* only guard: reject zero */
{ ... return -ENODEV; }
962 priv->lower.maxpoint = priv->device_caps.max_touches; /* up to 255,
no upper clamp */
...
645 framelen = ST7123_FRAME_HEADER_SIZE + /* 4 */
lower->maxpoint * ST7123_TOUCH_AREA_SIZE; /* maxpoint * 7 */
648 st7123_read_sequential(dev, ST7123_TOUCH_ADV_TOUCH_INFO,
(uint8_t *) &dev->frame, framelen); /* SINK */
```
`st7123_read_sequential()` → `i2c_writeread(dev->i2c, ..., value, count)`
(`:571`) reads
`framelen` device bytes straight into `dev->frame`.
The destination `struct st7123_frame_t dev->frame` is 74 bytes
(`adv_touch_info(1) + reserved(1) + gesture(1) + keys(1) + areas[10]*7`;
`st7123.c:221-228`,
`ST7123_MAX_TOUCH_AREAS=10`, `ST7123_TOUCH_AREA_SIZE=7`), inside the
heap-allocated `dev`
(`kmm_zalloc`, `:916`). With `max_touches = 0xff`, `framelen = 4 + 255*7 =
1789`, i.e. **1715 bytes
written past the `frame` field**. Note this is 1715 bytes past the *field*,
not past the whole
allocation: `frame` is an embedded member of the larger heap-allocated
`dev`, so the first ~300 of
those bytes still land **inside** `dev` (corrupting sibling state —
`device_caps`, `lastx[10]`,
`lasty[10]`, `sample_buf[]`, etc.), and only the remaining ~1400-byte tail
beyond `sizeof(*dev)` is a
true out-of-bounds heap write. Both effects are memory corruption; the
earlier phrasing "1715-byte
heap overflow" overstated the portion that actually escapes the allocation.
Secondary OOB writes in the same worker when `maxpoint > 10`:
`dev->lastx[i]`/`dev->lasty[i]`
(`:667-668`, `int16_t[10]`) and `sample->point[npoints++]` (`:680`,
`sample_buf` sized
`SIZEOF_TOUCH_SAMPLE_S(10)`).
`st7123_probe_device()` runs during `st7123_register()` at boot/driver init;
the oversized frame
read fires on the **first touch interrupt** (`st7123_interrupt` → HPWORK →
`st7123_data_worker`).
No userspace `open()`/`read()` is required — the corruption is driven
entirely by the I2C
peripheral. Gated by `CONFIG_INPUT_ST7123`. The bug is present in **all
build configurations**
(independent of `CONFIG_DEBUG_ASSERTIONS`).
**Why this is a bug (smoking gun): the clamp is promised in a comment but
never implemented.**
The header comment claims the value is clamped:
```c
/* drivers/input/st7123.c:79-82 */
* Maximum number of touch areas this driver is able to report. The value
* read back from ST7123_TOUCH_MAX_TOUCHES is clamped to this limit so that a
* bogus register value can never overrun the frame or sample buffers.
#define ST7123_MAX_TOUCH_AREAS 10
```
No `min()` / `if (max_touches > ST7123_MAX_TOUCH_AREAS)` exists anywhere in
the file. Sibling
touch drivers do clamp: `drivers/input/ft5x06.c:243` gates on
`if (sample->tdstatus <= FT5X06_MAX_TOUCHES)` and `ft5x06.c:500-503` clamps
`ntouches` to the
caller buffer; `drivers/input/mxt.c:1017` bounds its report-id index. ST7123
omits the
equivalent upper clamp entirely.
Steps:
1. Build with `CONFIG_INPUT_ST7123`.
2. A malicious/compromised ST7123 controller reports `max_touches = 0xff` in
its capability block.
3. On the first touch interrupt the driver issues a burst read of `framelen
= 4 + 255*7 = 1789`
bytes into the 74-byte `dev->frame`, overflowing by 1715 bytes.
An AddressSanitizer build with `max_touches = 0xff` and the burst read into
the 74-byte frame
reports:
```
heap-buffer-overflow WRITE of size 1789
```
**Suggested fix:** actually implement the clamp the comment promises, at
registration (and/or in the
worker):
```c
if (priv->device_caps.max_touches > ST7123_MAX_TOUCH_AREAS)
{
priv->device_caps.max_touches = ST7123_MAX_TOUCH_AREAS;
}
```
### On which OS does this issue occur?
[OS: Linux]
### What is the version of your OS?
ubuntu 22
### 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]
