K-ANOY opened a new issue, #21112:
URL: https://github.com/apache/nuttx/issues/21112
### Description / Steps to reproduce the issue
In `drivers/net/lan9250.c` (LAN9250 Ethernet driver over SPI/QSPI) the
received frame length taken
from the RX Status FIFO is used as a copy length into `priv->pktbuf`,
bounded on the SPI path **only**
by an assert that compiles to nothing in the default build, and on the
QSPI/SQI path by **no capacity
check at all**. `priv` is a `static` global object, so this is an
out-of-bounds **write** into `.bss`
that first clobbers the structure members immediately following `pktbuf` (a
watchdog timer and several
work structures) and then adjacent globals. The attacker is a peer on the
same L2 network sending an
oversized frame, or a malicious/faulty LAN9250 PHY.
`lan9250_receive()` takes the received frame length from the LAN9250 RX
Status FIFO register and
passes it as the copy length, with only a non-zero check:
```c
/* drivers/net/lan9250.c:1747-1748 (lan9250_receive) */
regval = lan9250_get_reg(priv, LAN9250_RXSFR); /* RX Status FIFO
register */
pktlen = (regval & RXSFF_PL_M) >> RXSFF_PL_S; /* 14-bit length
field, 0..16383 */
if (pktlen) /* only a non-zero
check */
lan9250_recv_buffer(priv, priv->dev.d_buf, pktlen); /* buflen = pktlen
*/
```
The copy then runs into `priv->pktbuf`. On the **SPI** path the length is
bounded only by a
`DEBUGASSERT`; on the **QSPI/SQI** path there is no capacity check:
```c
/* drivers/net/lan9250.c:853 (inside #ifdef CONFIG_LAN9250_SPI) */
DEBUGASSERT(buflen <= sizeof(priv->pktbuf)); /* SPI path:
default-off assert */
...
SPI_RECVBLOCK(priv->spi, buffer, buflen); /* buffer =
priv->dev.d_buf = priv->pktbuf */
/* drivers/net/lan9250.c:881-899 (#else — QSPI/SQI path) */
meminfo.buflen = buflen; /* no capacity check
at all */
QSPI_MEMORY(priv->qspi, &meminfo);
```
`priv->pktbuf` is `uint32_t pktbuf[LAN9250_PKTBUF_SIZE / 4]` (`:212`), with
`LAN9250_PKTBUF_SIZE = LAN9250_ALIGN(MAX_NETDEV_PKTSIZE +
CONFIG_NET_GUARDSIZE + 4)` (`:142`). In the
default IPv4/Ethernet config (`CONFIG_NET_ETH_PKTSIZE = 590`,
`CONFIG_NET_GUARDSIZE = 2`) that is
`ALIGN(596) = 596` bytes — about one max Ethernet frame. Any ordinary frame
larger than ~596 bytes
(a normal 1514/1518-byte Ethernet frame already qualifies) therefore
overflows it. `pktlen` is a
14-bit RX-status register field (max 16383); that width is the hardware
status-field size, **not** a
demonstration that a peer can inject a 16 KB frame — the practically
reachable and sufficient case is
simply "frame > ~596 bytes".
**Memory class:** the driver state is a `static` global, not a heap
allocation:
```c
/* drivers/net/lan9250.c:230 */
static struct lan9250_driver_s g_lan9250;
/* :2440 / :2512 */ priv = &g_lan9250; /* no kmm_malloc/kmm_zalloc of the
driver state */
```
`pktbuf` (`:212`) is immediately followed in the struct by `txtout_timer`
(`struct wdog_s`),
`txtout_work` / `txpoll_work` / `irq_work` (`struct work_s`), and `dev`
(`:214-224`). So the overflow
is a **`.bss`/global-buffer overflow** that corrupts those sibling members
(watchdog + work queue
state) first, then neighbouring `.bss` objects — it is **not** a heap
overflow.
**Configuration dimension:**
- SPI build, `CONFIG_DEBUG_ASSERTIONS = n` (**default**): the guard is
compiled out → overflow.
- SPI build, `CONFIG_DEBUG_ASSERTIONS = y`: the assert fires → controlled
panic (DoS), no overflow.
- **QSPI/SQI build (`#else` path): there is no capacity check in any
configuration**, so enabling
assertions does **not** prevent the overflow on QSPI/SQI targets.
**Why this is a bug:** the length is a parameter at the sink
(`lan9250_recv_buffer`)
that originates in the caller (`lan9250_receive`), and the only guard
anywhere on the SPI path is the
default-off `DEBUGASSERT` at `:853` (and nothing at all on the QSPI path).
The caller's `if (pktlen)`
check is non-zero-only, with no upper bound against `MAX_NETDEV_PKTSIZE`
before handing the
device-reported length to the copy.
Steps:
1. Build the LAN9250 driver (SPI with default `CONFIG_DEBUG_ASSERTIONS`
unset, or any QSPI/SQI build).
2. A peer on the same L2 network sends a frame larger than ~596 bytes (an
ordinary full-size Ethernet
frame suffices), or a malicious/faulty LAN9250 PHY reports an oversized
RX-status length.
3. The copy writes `pktlen` bytes into the 596-byte `g_lan9250.pktbuf`,
overflowing into the adjacent
watchdog/work members and `.bss`.
An AddressSanitizer build of the copy with the real buffer sizing (as a
596-byte global region)
reports a `global-buffer-overflow WRITE`.
**Suggested fix:** add an always-compiled check before
`lan9250_recv_buffer`, and/or replace the
`DEBUGASSERT`, and add the equivalent check on the QSPI/SQI path:
```c
if (pktlen > MAX_NETDEV_PKTSIZE)
{
/* drop oversized frame */
return;
}
```
and in `lan9250_recv_buffer` replace the SPI `DEBUGASSERT` with `if (buflen
> sizeof(priv->pktbuf)) return;`
and add the same guard before the QSPI `QSPI_MEMORY` transfer.
### 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]
