[PATCH v3 0/3] net: nfs: bound server-supplied lengths in READ and READLINK replies

Shahriyar Jalayeri <[email protected]>
Newsgroups org.u-boot-project.lists.u-boot
Message-ID <[email protected]>
A malicious NFS server can return replies whose 32-bit lengths are crafted
to defeat the client's bounds checks.

nfs_read_reply() keeps the READ length in a signed int. On LP64 a value
with the top bit set is negative, the bounds check passes, and
store_block() then hands a ~2 GB length to memcpy(), which reads past the
1152-byte reply buffer on the stack and writes past image_load_addr.

nfs_readlink_reply() has the same signed-length flaw. A length of -1 slips
past the destination bound as pathlen - 1 and drives a memcpy() off
nfs_path_buff. The bound is also measured from the reply header rather than
from the symlink data, so a large positive length reads a few bytes past
the received reply. A server reaches this handler by answering the READ
with an ISDIR status, which moves the client into the readlink state.

Both handlers are shared by the classic and lwIP NFS clients through
nfs_pkt_recv().

Patch 1 makes the READ length unsigned and bounds it against the received
packet and NFS_READ_SIZE. Patch 2 does the same for the readlink length
and its destination bound. Patch 3 enables CONFIG_CMD_NFS in the sandbox
config and adds DM regression tests that drive nfs_pkt_recv() with crafted
replies.

A reproducer is available on request.

Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
Changes in v3, all following Jerome Forissier's review:
- nfs_read_reply() and nfs_readlink_reply(): keep the server-supplied
  length in an unsigned int and bound it against the received packet with
  the subtraction ordered so it cannot wrap, dropping the explicit
  negative check.
- nfs_read_reply(): take the NFSv3 length from the opaque data_size field
  that prefixes the returned bytes.
- test: also set the data length field that nfs_read_reply() now reads.

Changes in v2:
- Add a fix for the same signed-length flaw in nfs_readlink_reply(),
  rejecting a negative length and measuring the bound from the symlink
  data.
- Enable CONFIG_CMD_NFS in sandbox_defconfig so the regression tests are
  built and run under sandbox; the v1 test was skipped in CI.
- Add a readlink regression test alongside the read one.

---
Shahriyar Jalayeri (3):
      net: nfs: bound the length of an NFS read reply
      net: nfs: bound the length of an NFS readlink reply
      test: dm: nfs: add regression tests for the NFS reply-length checks

 configs/sandbox_defconfig |  1 +
 net/nfs-common.c          | 32 ++++++++++-------
 test/dm/Makefile          |  1 +
 test/dm/nfs.c             | 91 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 12 deletions(-)
---
base-commit: baa64b2f892890f00a377eac4a3e685472bb56b5
change-id: 20260811-nfs-oob-fix-31c433a22c91

Best regards,
--  
Shahriyar Jalayeri <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.