[PATCH v2 1/2] fs/squashfs: fix integer overflow in directory table allocation

Shahriyar Jalayeri <[email protected]>
Newsgroups gmane.comp.boot-loaders.u-boot
Message-ID <[email protected]>
sqfs_read_directory_table() allocates the directory table with
malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE). metablks_count is an
int and SQFS_METADATA_BLOCK_SIZE is 8192, so the multiply is evaluated in
int and wraps for metablks_count >= 2^19. metablks_count comes from the
attacker-controlled superblock (sqfs_count_metablks() grows it by one per
2-byte metadata header), so a crafted image under-allocates the buffer
while the fill loop still writes metablks_count metadata blocks into it,
a heap out-of-bounds write. It is reached by listing or reading the image
(sqfsls / sqfsload). The position list allocation on the next line has the
same unchecked-multiply shape.

Size both allocations with __builtin_mul_overflow() and reject the image
on overflow, as the disk-read buffers earlier in the same function already
do. Set the error return when either allocation fails so the caller does
not proceed with a NULL directory table.

Fixes: c51006130370 ("fs/squashfs: new filesystem")
Signed-off-by: Shahriyar Jalayeri <[email protected]>
---
 fs/squashfs/sqfs.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c
index 0768fc4a7b2..3aadcdd36ec 100644
--- a/fs/squashfs/sqfs.c
+++ b/fs/squashfs/sqfs.c
@@ -852,13 +852,28 @@ static int sqfs_read_directory_table(unsigned char **dir_table, u32 **pos_list)
 	if (metablks_count < 1)
 		goto out;
 
-	*dir_table = malloc(metablks_count * SQFS_METADATA_BLOCK_SIZE);
-	if (!*dir_table)
+	if (__builtin_mul_overflow(metablks_count, SQFS_METADATA_BLOCK_SIZE,
+				   &buf_size)) {
+		metablks_count = -1;
+		goto out;
+	}
+
+	*dir_table = malloc(buf_size);
+	if (!*dir_table) {
+		metablks_count = -1;
+		goto out;
+	}
+
+	if (__builtin_mul_overflow(metablks_count, sizeof(u32), &buf_size)) {
+		metablks_count = -1;
 		goto out;
+	}
 
-	*pos_list = malloc(metablks_count * sizeof(u32));
-	if (!*pos_list)
+	*pos_list = malloc(buf_size);
+	if (!*pos_list) {
+		metablks_count = -1;
 		goto out;
+	}
 
 	ret = sqfs_get_metablk_pos(*pos_list, dtb, table_offset,
 				   metablks_count);

-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.