[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]>