#60 VLA Overflows

There seems to be a stack overflow and an integer overflow in read_fragment_table_4.

The first problem overflows the bytes variable, so that the allocation of fragments_bytes[] has an erroneous size.

int bytes = SQUASHFS_FRAGMENT_BYTES(sBlk.s.fragments);
...
fragment_table = malloc(bytes);

If we fix this by making the variable size_t, we run into an unrelated problem in which the stack VLA allocation of fragment_table_index[] can easily exceed RLIMIT_STACK.

long long fragment_table_index[indexes];

In the case of my system, the RLIMIT_STACK is 8388608 bytes, and VLA is asking for 15728648 bytes. This is what I believe ultimately leads to the stack overflow. Plus the stack probably already has a bunch of other things, and I don't know a portable way to check how much stack space is available.

Afterwards, the disastrous call to read_fs_bytes is made, which initiates the transfer from the squashfs image to the stack. At this stage, a stack overflow appears to be in full effect.

 res = read_fs_bytes(fd, sBlk.s.fragment_table_start,
         SQUASHFS_FRAGMENT_INDEX_BYTES(sBlk.s.fragments),
         fragment_table_index);

ASAN Output

Parallel unsquashfs: Using 8 processors
ASAN:SIGSEGV
=================================================================
==8221==ERROR: AddressSanitizer: stack-overflow on address 0x7ffef3ae9608 (pc 0x000000559011 bp 0x7ffef49e9670 sp 0x7ffef3ae9610 T0)
    #0 0x559010 in read_fragment_table_4 /home/septimus/vr/squashfs-vr/squashfs-tools/unsquash-4.c:40:9
    #1 0x525073 in main /home/septimus/vr/squashfs-vr/squashfs-tools/unsquashfs.c:2763:5
    #2 0x7fb56c533a3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x20a3f)
    #3 0x418468 in _start (/home/septimus/vr/squashfs-vr/squashfs-tools/unsquashfs+0x418468)
SUMMARY: AddressSanitizer: stack-overflow /home/septimus/vr/squashfs-vr/squashfs-tools/unsquash-4.c:40:9 in read_fragment_table_4
==8221==ABORTING

Perhaps we should avoid using VLA altogether, and allocate fragment_table_index to the heap? It's likely that the fragment_table_index[] can be a lot bigger than 8388608 bytes, according to the squashfs specification.

This problem is present in other read_fragment_table_N functions, and in in the original squashfs-tools.

This pull request is an example implementation of the recommended fix for unsquash-4, but I don't have enough test vectors to verify it doesn't break anything. All code that uses VLA should probably be converted to use the heap instead.

Please let me know what you think.

Thanks.

[Anonymous]

Please look at the pull request @Git: https://github.com/devttys0/sasquatch/pull/5