[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: sta...@vger.kernel.org
Signed-off-by: Piyush Paliwal <piyushthe...@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