[PATCH v3 1/3] net: nfs: bound the length of an NFS read reply

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