[PATCH v3 2/3] net: nfs: bound the length of an NFS readlink reply
Shahriyar Jalayeri <[email protected]>
| Newsgroups | org.u-boot-project.lists.u-boot |
|---|---|
| Message-ID | <[email protected]> |
nfs_readlink_reply() takes the symlink length from the server and
memcpy()s that many bytes into nfs_path_buff[]. It was kept in a signed
int and bounded with:
if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
return -NFS_RPC_DROP;
A negative rlen makes the sum smaller than len, so the check passes; rlen
is then used as an unsigned size_t in memcpy(), and in the relative-symlink
branch pathlen + rlen also stays below the buffer size, so a length of -1
drives a memcpy() off nfs_path_buff. The bound is also measured from the
reply header rather than from the symlink data, which begins a few words
later, so a large positive length reads past the end of the received
reply.
A malicious server answers the READ with an ISDIR status to move the
client into the readlink state, then returns such a reply.
Read the length into an unsigned int, bound it against the received packet
measured from the symlink data, and check it against the destination
buffer with the subtraction ordered so it cannot wrap.
Fixes: cf3a4f1e86ec ("CVE-2019-14195: nfs: fix unbounded memcpy with unvalidated length at nfs_readlink_reply")
Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
net/nfs-common.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/nfs-common.c b/net/nfs-common.c
index 91ae7a43b8c..c8bc101cda1 100644
--- a/net/nfs-common.c
+++ b/net/nfs-common.c
@@ -642,7 +642,8 @@ static int nfs3_get_attributes_offset(uint32_t *data)
static int nfs_readlink_reply(uchar *pkt, unsigned int len)
{
struct rpc_t rpc_pkt;
- int rlen;
+ u32 rlen;
+ size_t data_offset;
int nfsv3_data_offset = 0;
memcpy((unsigned char *)&rpc_pkt, pkt, len);
@@ -665,16 +666,20 @@ static int nfs_readlink_reply(uchar *pkt, unsigned int len)
/* new path length */
rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
+ data_offset = (uchar *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset] -
+ (uchar *)&rpc_pkt;
- if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
+ /* reject a length that runs past the received packet */
+ if (data_offset > len || rlen > len - data_offset)
return -NFS_RPC_DROP;
if (*((char *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset]) != '/') {
- int pathlen;
+ size_t pathlen;
strcat(nfs_path, "/");
pathlen = strlen(nfs_path);
- if (pathlen + rlen >= sizeof(nfs_path_buff))
+ if (pathlen >= sizeof(nfs_path_buff) ||
+ rlen >= sizeof(nfs_path_buff) - pathlen)
return -NFS_RPC_DROP;
memcpy(nfs_path + pathlen,
(uchar *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset],
--
2.43.0