[PATCH v1 1/2] fs/squashfs: bound fragment table accesses in sqfs_frag_lookup()

[Pranav Rajendran <pranavkasthuri@gmail.com>]

sqfs_frag_lookup() validates its fragment index only against
sblk->fragments, which is read from the image superblock and is
therefore under the control of whoever supplies the image. Every
buffer access derived from that index is then made without checking
it against the size of the buffer actually read from the device:

 - the fragment index table entry at 'table_offset + block *
   sizeof(u64)' can be read past the end of 'table', as 'block' is
   inode_fragment_index / SQFS_MAX_ENTRIES and has no upper bound;

 - the metadata block header and payload are read from
   'metadata_buffer' at 'table_offset', but that buffer is sized from
   start_block, which comes from the unvalidated index table entry
   above. A start_block just below sblk->fragment_table_start yields a
   single-block buffer while SQFS_METADATA_SIZE(header) may be up to
   SQFS_METADATA_BLOCK_SIZE, so both the decompression source and the
   memcpy() source can run past the end of the buffer;

 - 'entries' is allocated with SQFS_METADATA_BLOCK_SIZE bytes but only
   partially filled, so entries[offset] can read uninitialised heap
   memory when the metadata block holds fewer than offset + 1 entries.

Compute the size of both buffers explicitly, rejecting the
multiplication overflow the way sqfs_read_directory_table() already
does, and check each access against it. Also track how much of
'entries' was populated and reject an index beyond that.

A crafted SquashFS image can trigger all three cases, either crashing
U-Boot or feeding adjacent heap contents into the fragment entry that
the following data read is based on.

Fixes: c51006130370 ("fs/squashfs: new filesystem")
Signed-off-by: Pranav Rajendran <pranavkasthuri@gmail.com>
---
 fs/squashfs/sqfs.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 55fbe1bcc1a..cd88923521e 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -109,6 +109,7 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	unsigned char *metadata_buffer, *metadata, *table;
 	struct squashfs_fragment_block_entry *entries;
 	struct squashfs_super_block *sblk = ctxt.sblk;
+	size_t table_size, metadata_size, valid_len;
 	unsigned long dest_len;
 	int block, offset, ret;
 	u16 header;
@@ -133,7 +134,12 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	start /= ctxt.cur_dev->blksz;
 
 	/* Allocate a proper sized buffer to store the fragment index table */
-	table = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
+	if (__builtin_mul_overflow(n_blks, ctxt.cur_dev->blksz, &table_size)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	table = malloc_cache_aligned(table_size);
 	if (!table) {
 		ret = -ENOMEM;
 		goto out;
@@ -147,6 +153,16 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	block = SQFS_FRAGMENT_INDEX(inode_fragment_index);
 	offset = SQFS_FRAGMENT_INDEX_OFFSET(inode_fragment_index);
 
+	/*
+	 * 'inode_fragment_index' is only checked against sblk->fragments, which
+	 * is itself read from the image, so the resulting index may point past
+	 * the fragment index table that was actually read from the device.
+	 */
+	if (table_offset + ((u64)block + 1) * sizeof(u64) > table_size) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/*
 	 * Get the start offset of the metadata block that contains the right
 	 * fragment block entry
@@ -158,7 +174,13 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	n_blks = sqfs_calc_n_blks(cpu_to_le64(start_block),
 				  sblk->fragment_table_start, &table_offset);
 
-	metadata_buffer = malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
+	if (__builtin_mul_overflow(n_blks, ctxt.cur_dev->blksz,
+				   &metadata_size)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	metadata_buffer = malloc_cache_aligned(metadata_size);
 	if (!metadata_buffer) {
 		ret = -ENOMEM;
 		goto out;
@@ -170,6 +192,11 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 	}
 
 	/* Every metadata block starts with a 16-bit header */
+	if (table_offset + SQFS_HEADER_SIZE > metadata_size) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	header = get_unaligned_le16(metadata_buffer + table_offset);
 	metadata = metadata_buffer + table_offset + SQFS_HEADER_SIZE;
 
@@ -183,6 +210,16 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 		goto out;
 	}
 
+	/*
+	 * The metadata block's payload is read straight out of
+	 * 'metadata_buffer', so it has to fit in what was read from the device.
+	 */
+	if (table_offset + SQFS_HEADER_SIZE + SQFS_METADATA_SIZE(header) >
+	    metadata_size) {
+		ret = -EINVAL;
+		goto out;
+	}
+
 	entries = malloc(SQFS_METADATA_BLOCK_SIZE);
 	if (!entries) {
 		ret = -ENOMEM;
@@ -198,8 +235,17 @@ static int sqfs_frag_lookup(u32 inode_fragment_index,
 			ret = -EINVAL;
 			goto out;
 		}
+
+		valid_len = dest_len;
 	} else {
 		memcpy(entries, metadata, SQFS_METADATA_SIZE(header));
+		valid_len = SQFS_METADATA_SIZE(header);
+	}
+
+	/* Only the part of 'entries' that was actually filled in is usable */
+	if (((u64)offset + 1) * sizeof(*entries) > valid_len) {
+		ret = -EINVAL;
+		goto out;
 	}
 
 	*e = entries[offset];
-- 
2.50.1 (Apple Git-155)