[PATCH 2/2] fs/squashfs: bound fragment offset/size in sqfs_read_nest()

[Piyush Paliwal]

When reading a fragment-backed file, sqfs_read_nest() copies the file
data out of the fragment block with:

  memcpy(buf + *actread, &fragment_block[finfo.offset],
         finfo.size - *actread);

finfo.offset (the fragment's byte offset) and finfo.size come straight
from the on-disk inode and are never validated against the fragment
block length. Unlike the data-block loop above it, this path does not
clamp the source span, so a crafted inode makes the memcpy read past the
fragment buffer -> out-of-bounds heap read. The leaked bytes are copied
into the user-visible load buffer (information disclosure) or fault.
This affects both the compressed (dest_len bytes) and the uncompressed
(table_size bytes) fragment cases.

Validate finfo.offset and the copy length against the available fragment
data before each memcpy and reject malformed inodes.

Found by fuzzing the sandbox (CONFIG_ASAN) sqfsload with mutated images:
before, SEGV in sqfs_read_nest() at the fragment memcpy; after, malformed
images are rejected and valid fragmented files still load correctly.

Fixes: 0008d8086649 ("fs/squashfs: fix reading of fragmented files")
Cc: stable at vger.kernel.org
Signed-off-by: Piyush Paliwal <piyushthepal at gmail.com>
---
 fs/squashfs/sqfs.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 3548b5e07e2..6e7723669d4 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -1630,6 +1630,19 @@ static int sqfs_read_nest(const char *filename, void *buf, loff_t offset,
 			goto out;
 		}
 
+		/*
+		 * finfo.offset and finfo.size come from the on-disk inode and
+		 * must not let the copy read past the decompressed fragment
+		 * block (dest_len bytes).
+		 */
+		if (finfo.size < (size_t)*actread ||
+		    finfo.offset > dest_len ||
+		    finfo.size - *actread > dest_len - finfo.offset) {
+			free(fragment_block);
+			ret = -EINVAL;
+			goto out;
+		}
+
 		memcpy(buf + *actread, &fragment_block[finfo.offset], finfo.size - *actread);
 		*actread = finfo.size;
 
@@ -1638,6 +1651,17 @@ static int sqfs_read_nest(const char *filename, void *buf, loff_t offset,
 	} else if (finfo.frag && !finfo.comp) {
 		fragment_block = (void *)fragment + table_offset;
 
+		/*
+		 * Same check for the uncompressed fragment: the readable data
+		 * is table_size bytes starting at table_offset within fragment.
+		 */
+		if (finfo.size < (size_t)*actread ||
+		    finfo.offset > table_size ||
+		    finfo.size - *actread > table_size - finfo.offset) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		memcpy(buf + *actread, &fragment_block[finfo.offset], finfo.size - *actread);
 		*actread = finfo.size;
 	}
-- 
2.41.0