94xhn opened a new pull request, #19390:
URL: https://github.com/apache/nuttx/pull/19390
## Summary
Fixes #14743. In `udp_input()`'s broadcast/multicast fan-out loop
(`net/udp/udp_input.c`), each iteration calls `netdev_iob_replace(dev, iob)` to
swap in a freshly cloned iob before handing the packet to the next matching
`SO_REUSEADDR` connection. `netdev_iob_replace()` unconditionally sets
`dev->d_len = iob->io_pktlen` (`net/netdev/netdev_iob.c:149`), which is the
full frame length (IP + UDP headers + payload) — undoing the `dev->d_len -=
udpiplen` done once before the loop to strip the headers off for
`udp_input_conn()`.
As a result, every connection after the first sees a `d_len` that is
`udpiplen` (IP+UDP header length, e.g. 28 bytes for IPv4) too large. This value
flows straight into `udp_datahandler()` as `buflen`
(`net/udp/udp_callback.c:256`, `int buflen = dev->d_len;`) and gets stored as
the queued packet's declared length in the connection's read-ahead iob chain.
Once more than one such oversized entry has queued up in the same chain, the
consumer (`udp_readahead()` in `net/udp/udp_recvfrom.c`) parses the following
entry's metadata starting at the wrong offset, so whatever byte happens to land
on `src_addr_size` gets trusted as-is. That single byte (0-255) is then used as
the length in `iob_copyout(srcaddr, iob, src_addr_size, offset)`
(`udp_recvfrom.c:218`), which fills a fixed-size stack buffer (`uint8_t
srcaddr[sizeof(struct sockaddr_in6)]`) with no bounds check outside a
`DEBUGASSERT` — compiled out in release builds. An oversized value overflows
that stack buffer.
## Impact
Affects any target with `CONFIG_NET_SOCKOPTS` + `CONFIG_NET_BROADCAST` where
two or more `SO_REUSEADDR` UDP sockets are bound to the same port and receive
broadcast/multicast traffic — a normal, non-adversarial configuration (the
issue reporter hit it without any malicious input). Security-relevant: stack
buffer overflow in the network receive path.
## Fix
Re-apply the same `-= udpiplen` header-stripping after each
`netdev_iob_replace()` call in the loop, matching what's already done once
before the loop for the first connection — a 1-line change.
## Testing
I don't have a NuttX `sim` build environment set up in my sandbox (no
WSL/Linux host available to me), so I couldn't produce `sim:nsh`/`sim:netnsh`
execution logs. What I did instead:
1. Traced the exact value flow through `net/udp/udp_input.c` →
`net/netdev/netdev_iob.c` → `net/udp/udp_callback.c` → `net/udp/udp_recvfrom.c`
against current `master`, confirming every intermediate read/write of
`d_len`/`buflen`/`datalen`/`src_addr_size` cited above by line number.
2. Extracted just the relevant field-level arithmetic (`d_len` tracking
across the fan-out loop, `netdev_iob_replace()` copied verbatim) into a
standalone host-C reproduction, compiled and run with plain `gcc` (no NuttX
build needed since this part has no OS dependency):
- **Before fix**: conn1 sees `d_len` = 1014 (correct), conn2 and conn3
both see `d_len` = 1042 — exactly 28 (`udpiplen`) too large.
- **After fix** (`-DFIXED`, i.e. this patch's one added line): all three
connections see the correct `d_len` = 1014.
3. Cross-checked against the issue reporter's own finding: they
independently added a `DEBUGASSERT()` to `udp_readahead()` that fires, and
observed "`the value of dev->d_len matched the size of the application data
before udp_input_conn() was called the 1st time... The second time (for the
second [connection])`" — the same first-vs-second-connection `d_len` mismatch
my trace and repro reproduce.
Happy to run the real `sim:netnsh` reproduction and post logs if a
maintainer can point me to (or confirm) the right multi-`SO_REUSEADDR`-socket
test setup, or if this needs stronger evidence before merge.
--
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]
