nfs_read_reply() takes the length of the returned data from the server
and hands it to store_block(), which memcpy()s that many bytes out of the
1152-byte rpc_pkt stack buffer to image_load_addr.
The length was kept in a signed int and bounded with:
if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) >
len)
return -9999;
On an LP64 target the pointer subtraction is a 64-bit ptrdiff_t, so a
length with the top bit set makes rlen negative, the sum stays below len
and the check passes. store_block() then takes rlen as an unsigned int,
so 0x80000000 becomes a ~2 GB copy that runs off both buffers. The bound
is also measured from the reply header rather than from the data, which
begins several words later, so a large positive length still reads past
the end of rpc_pkt.
Read the length into an unsigned int so it can never be negative, reject
anything larger than NFS_READ_SIZE (the most a read requests), and bound
it against the received packet measured from the start of the data. Take
the NFSv3 length from the opaque data_size field that prefixes the
returned bytes, which is what store_block() copies.
Both the classic and the lwIP NFS clients reach this through
nfs_pkt_recv(), so the single check covers both.
Fixes: aa207cf3a6d6 ("CVE-2019-14194/CVE-2019-14198: nfs: fix unbounded memcpy
with a failed length check at nfs_read_reply")
Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
net/nfs-common.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/net/nfs-common.c b/net/nfs-common.c
index 72d8fd823e3..91ae7a43b8c 100644
--- a/net/nfs-common.c
+++ b/net/nfs-common.c
@@ -694,7 +694,8 @@ static int nfs_readlink_reply(uchar *pkt, unsigned int len)
static int nfs_read_reply(uchar *pkt, unsigned int len)
{
struct rpc_t rpc_pkt;
- int rlen;
+ u32 rlen;
+ size_t data_offset;
uchar *data_ptr;
memcpy(&rpc_pkt.u.data[0], pkt, sizeof(rpc_pkt.u.reply));
@@ -728,17 +729,19 @@ static int nfs_read_reply(uchar *pkt, unsigned int len)
int nfsv3_data_offset =
nfs3_get_attributes_offset(rpc_pkt.u.reply.data);
- /* count value */
- rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
- /* Skip unused values :
- * EOF: 32 bits value,
- * data_size: 32 bits value,
- */
+ /* Skip count and EOF, read data_size from opaque data */
+ rlen = ntohl(rpc_pkt.u.reply.data[3 + nfsv3_data_offset]);
data_ptr = (uchar *)
&rpc_pkt.u.reply.data[4 + nfsv3_data_offset];
}
- if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) >
len)
+ /* reject a length larger than a read requests */
+ if (rlen > NFS_READ_SIZE)
+ return -9999;
+
+ /* reject a length that runs past the received packet */
+ data_offset = data_ptr - (uchar *)&rpc_pkt;
+ if (data_offset > len || rlen > len - data_offset)
return -9999;
if (store_block(data_ptr, nfs_offset, rlen))
--
2.43.0
