Skip to content
Permalink
Browse files Browse the repository at this point in the history
unsquashfs-4: Add more sanity checks + fix CVE-2015-4645/6
Add more filesystem table sanity checks to Unsquashfs-4 and
also properly fix CVE-2015-4645 and CVE-2015-4646.

The CVEs were raised due to Unsquashfs having variable
oveflow and stack overflow in a number of vulnerable
functions.

The suggested patch only "fixed" one such function and fixed
it badly, and so it was buggy and introduced extra bugs!

The suggested patch was not only buggy, but, it used the
essentially wrong approach too.  It was "fixing" the
symptom but not the cause.  The symptom is wrong values
causing overflow, the cause is filesystem corruption.
This corruption should be detected and the filesystem
rejected *before* trying to allocate memory.

This patch applies the following fixes:

1. The filesystem super-block tables are checked, and the values
   must match across the filesystem.

   This will trap corrupted filesystems created by Mksquashfs.

2. The maximum (theorectical) size the filesystem tables could grow
   to, were analysed, and some variables were increased from int to
   long long.

   This analysis has been added as comments.

3. Stack allocation was removed, and a shared buffer (which is
   checked and increased as necessary) is used to read the
   table indexes.

Signed-off-by: Phillip Lougher <phillip@squashfs.org.uk>
  • Loading branch information
plougher committed Jul 16, 2019
1 parent 5883059 commit f95864a
Show file tree
Hide file tree
Showing 3 changed files with 239 additions and 54 deletions.
57 changes: 43 additions & 14 deletions squashfs-tools/read_xattrs.c
Expand Up @@ -150,7 +150,16 @@ static int read_xattr_entry(struct xattr_list *xattr,
*/
int read_xattrs_from_disk(int fd, struct squashfs_super_block *sBlk, int flag, long long *table_start)
{
int res, bytes, i, indexes, index_bytes, ids;
/*
* Note on overflow limits:
* Size of ids (id_table.xattr_ids) is 2^32 (unsigned int)
* Max size of bytes is 2^32*16 or 2^36
* Max indexes is (2^32*16)/8K or 2^23
* Max index_bytes is ((2^32*16)/8K)*8 or 2^26 or 64M
*/
int res, i, indexes, index_bytes;
unsigned int ids;
long long bytes;
long long *index, start, end;
struct squashfs_xattr_table id_table;

Expand All @@ -170,24 +179,44 @@ int read_xattrs_from_disk(int fd, struct squashfs_super_block *sBlk, int flag, l

SQUASHFS_INSWAP_XATTR_TABLE(&id_table);

if(flag) {
/*
* id_table.xattr_table_start stores the start of the compressed xattr
* * metadata blocks. This by definition is also the end of the previous
* filesystem table - the id lookup table.
*/
/*
* Compute index table values
*/
ids = id_table.xattr_ids;
xattr_table_start = id_table.xattr_table_start;
index_bytes = SQUASHFS_XATTR_BLOCK_BYTES((long long) ids);
indexes = SQUASHFS_XATTR_BLOCKS((long long) ids);

/*
* The size of the index table (index_bytes) should match the
* table start and end points
*/
if(index_bytes != (sBlk->bytes_used - (sBlk->xattr_id_table_start + sizeof(id_table)))) {
ERROR("read_xattrs_from_disk: Bad xattr_ids count in super block\n");
return 0;
}

/*
* id_table.xattr_table_start stores the start of the compressed xattr
* metadata blocks. This by definition is also the end of the previous
* filesystem table - the id lookup table.
*/
if(table_start != NULL)
*table_start = id_table.xattr_table_start;

/*
* If flag is set then return once we've read the above
* table_start. That value is necessary for sanity checking,
* but we don't actually want to extract the xattrs, and so
* stop here.
*/
if(flag)
return id_table.xattr_ids;
}

/*
* Allocate and read the index to the xattr id table metadata
* blocks
*/
ids = id_table.xattr_ids;
xattr_table_start = id_table.xattr_table_start;
index_bytes = SQUASHFS_XATTR_BLOCK_BYTES(ids);
indexes = SQUASHFS_XATTR_BLOCKS(ids);
index = malloc(index_bytes);
if(index == NULL)
MEM_ERROR();
Expand All @@ -203,7 +232,7 @@ int read_xattrs_from_disk(int fd, struct squashfs_super_block *sBlk, int flag, l
* Allocate enough space for the uncompressed xattr id table, and
* read and decompress it
*/
bytes = SQUASHFS_XATTR_BYTES(ids);
bytes = SQUASHFS_XATTR_BYTES((long long) ids);
xattr_ids = malloc(bytes);
if(xattr_ids == NULL)
MEM_ERROR();
Expand All @@ -213,7 +242,7 @@ int read_xattrs_from_disk(int fd, struct squashfs_super_block *sBlk, int flag, l
bytes & (SQUASHFS_METADATA_SIZE - 1);
int length = read_block(fd, index[i], NULL, expected,
((unsigned char *) xattr_ids) +
(i * SQUASHFS_METADATA_SIZE));
((long long) i * SQUASHFS_METADATA_SIZE));
TRACE("Read xattr id table block %d, from 0x%llx, length "
"%d\n", i, index[i], length);
if(length == 0) {
Expand Down

0 comments on commit f95864a

Please sign in to comment.