From fc557ef9fe116b3245265629cf1d3ab4ac7a480c Mon Sep 17 00:00:00 2001 From: Pranav Rajendran Date: Sat, 15 Aug 2026 23:01:14 +0100 Subject: fs/squashfs: bound fragment table accesses in sqfs_frag_lookup() 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 Reviewed-by: Richard Genoud --- 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]; -- cgit v1.3.1 From e007b43a272edae8a63ad66f0151ceaf8cf003c1 Mon Sep 17 00:00:00 2001 From: Pranav Rajendran Date: Sat, 15 Aug 2026 23:01:15 +0100 Subject: fs/squashfs: bound the offset returned by sqfs_dir_offset() Commit 57e0bb7bf00d ("fs/squashfs: add sqfs_dir_offset() error checks") made sqfs_search_dir() reject negative returns from sqfs_dir_offset(), but the positive range is still unbounded. Both parts of the returned offset come from the image: 'offset' is a 16-bit inode field used verbatim, and the matched metadata block index may be the last one in m_list, in which case the returned block (j + 1) is one past the end of the directory table. The callers use the result to index dirs->dir_table[], which sqfs_read_directory_table() allocates as m_count metadata blocks, and then memcpy() a directory header out of it. A crafted image can therefore read up to 64 KiB past the end of that allocation. Reject an inode offset that cannot address a decompressed metadata block, and verify that the resulting directory header lies entirely within the directory table. The existing 'offset < 0' test is dropped: 'offset' is assigned from get_unaligned_le16() and so is never negative, meaning the test never fired. The new upper bound covers what it was meant to catch. Fixes: c51006130370 ("fs/squashfs: new filesystem") Signed-off-by: Pranav Rajendran Reviewed-by: Richard Genoud --- fs/squashfs/sqfs_dir.c | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index ed83c90682f..3908d1380b3 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -32,6 +32,7 @@ int sqfs_dir_offset(void *dir_i, u32 *m_list, int m_count) struct squashfs_base_inode *base = dir_i; struct squashfs_ldir_inode *ldir; struct squashfs_dir_inode *dir; + u64 table_size, res; u32 start_block; int j, offset; @@ -51,20 +52,49 @@ int sqfs_dir_offset(void *dir_i, u32 *m_list, int m_count) return -EINVAL; } - if (offset < 0) + /* + * 'offset' is an offset into a decompressed metadata block, so it can + * never address past the end of one. + */ + if (offset >= SQFS_METADATA_BLOCK_SIZE) return -EINVAL; + if (m_count < 1) + return -EINVAL; + + /* The caller's directory table holds m_count decompressed blocks. */ + table_size = (u64)m_count * SQFS_METADATA_BLOCK_SIZE; + for (j = 0; j < m_count; j++) { if (m_list[j] == start_block) - return (++j * SQFS_METADATA_BLOCK_SIZE) + offset; + break; } - if (start_block == 0) - return offset; + if (j < m_count) { + /* + * m_list[j] is the position of the metadata block following + * block j, so a match means the directory starts in block + * j + 1. + */ + res = (u64)(j + 1) * SQFS_METADATA_BLOCK_SIZE + offset; + } else if (start_block == 0) { + res = offset; + } else { + printf("Error: invalid inode reference to directory table.\n"); + return -EINVAL; + } - printf("Error: invalid inode reference to directory table.\n"); + /* + * Callers use the return value to index the directory table and read a + * directory header from it, so the whole header must lie inside the + * table. + */ + if (res + SQFS_DIR_HEADER_SIZE > table_size) { + printf("Error: inode points past the end of the directory table.\n"); + return -EINVAL; + } - return -EINVAL; + return res; } bool sqfs_is_empty_dir(void *dir_i) -- cgit v1.3.1