fix: add bounds check before memcpy in rpmsg_sockif.c#19407
fix: add bounds check before memcpy in rpmsg_sockif.c#19407orbisai0security wants to merge 1 commit into
Conversation
|
@linguini1 why close the security fix without dissussion? |
|
@CV-Bowen please review whether the change is valid? |
linguini1
left a comment
There was a problem hiding this comment.
The author stated on a previous PR that there is a human in the loop, so I was mistaken. The word "autonomous" does not indicate that.
Please follow the contributing guidelines.
aaf4f61 to
a85441e
Compare
done. |
linguini1
left a comment
There was a problem hiding this comment.
Waiting for runtime testing as recommended by author.
Also, please add the "Assisted-by: " field to your commit (see contributing guide).
In rpmsg_socket_ept_cb(), conn->recvlen is supplied by the remote
peer inside the received message header and was used directly as
the length argument to memcpy() without being bounded by the actual
payload length. A malicious peer could craft a packet whose header
length field exceeds the real payload, causing a heap buffer overflow
into conn->recvdata.
Fix this by clamping conn->recvlen to the true payload size before
each memcpy() call, handling the SOCK_STREAM and SOCK_DGRAM paths
separately:
- SOCK_STREAM: conn->recvlen = MIN(conn->recvlen, len)
- SOCK_DGRAM: conn->recvlen = MIN(conn->recvlen,
len > sizeof(uint32_t) ? len - sizeof(uint32_t) : 0)
Assisted-by: Claude (Anthropic)
Signed-off-by: orbisai0security <mediratta01.pally@gmail.com>
a85441e to
66eed93
Compare
|
Added the Regarding runtime testing: we don't currently have access to RPMSG-capable hardware (e.g. STM32MP1 or i.MX8 with dual-core IPC). If a maintainer or reviewer with such hardware could validate the fix at runtime, that would be appreciated. The change is a pure bounds-clamp (MIN) applied before existing memcpy calls, so it cannot alter behavior for well-formed packets — only prevents overflow from malformed ones. |
Summary
`rpmsg_socket_ept_cb()` in `net/rpmsg/rpmsg_sockif.c` copies received
data into `conn->recvdata` using `conn->recvlen` as the copy length.
`conn->recvlen` is peer-supplied (from the message header) and was not
bounded against the actual payload length (`len`) before being passed
to `memcpy()`. A malicious remote peer could send a crafted RPMSG
packet with an inflated length field, causing a heap buffer overflow
in `conn->recvdata`.
This patch clamps `conn->recvlen` to the real payload size immediately
before each `memcpy()` call, separately for the SOCK_STREAM and
SOCK_DGRAM paths.
Impact
RPMSG peer. No API or ABI changes.
oversized length fields in malformed packets are now silently
truncated rather than overflowing the destination buffer.
Testing
Built for `rv-virt:nsh` (RISC-V QEMU) with no new warnings or errors:
```
tools/configure.sh rv-virt:nsh
make -j$(nproc)
```
Note: Full RPMSG socket runtime testing on real hardware (e.g.
STM32MP1 or i.MX8) is recommended before merging.