mirror of
git://slackware.nl/current.git
synced 2025-01-30 08:38:10 +01:00
77 lines
3.6 KiB
Diff
77 lines
3.6 KiB
Diff
|
From 6777e08cc38bc780d27c69c1d8c272867b74524f Mon Sep 17 00:00:00 2001
|
||
|
From: Giancarlo Canales Barreto <gcanalesb@me.com>
|
||
|
Date: Wed, 17 Jun 2015 00:22:19 -0400
|
||
|
Subject: [PATCH] Update unsquash-4.c
|
||
|
|
||
|
There seems to be a stack overflow in read_fragment_table_4 at via what seems to be an integer overflow. Still looking into this problem, it seems like two or three different problems combined.
|
||
|
|
||
|
The first problem overflows the bytes variable, so that the allocation is enormous.
|
||
|
```c
|
||
|
int bytes = SQUASHFS_FRAGMENT_BYTES(sBlk.s.fragments);
|
||
|
```
|
||
|
|
||
|
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.
|
||
|
```c
|
||
|
long long fragment_table_index[indexes];
|
||
|
```
|
||
|
|
||
|
In the case of my system, the RLIMIT_STACK is 8388608, and VLA is asking for 15728648. Plus the stack probably already has a bunch of other things. This is what I believe ultimately leads to the stack overflow.
|
||
|
|
||
|
Afterwards, the heap allocation seems to succeed, and the disastrous call to read_fs_bytes is made, which initiates transfer from the squashfs image to the stack. At this stage, a stack overflow appears to be in full effect.
|
||
|
|
||
|
```c
|
||
|
res = read_fs_bytes(fd, sBlk.s.fragment_table_start,
|
||
|
SQUASHFS_FRAGMENT_INDEX_BYTES(sBlk.s.fragments),
|
||
|
fragment_table_index);
|
||
|
```
|
||
|
This problem is also present in other read_fragment_table_N functions, and in in the original squashfs-tools.
|
||
|
|
||
|
```
|
||
|
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?
|
||
|
This pull request is an example implementation of the fix for unsquash-4, but I don't have enough test vectors to verify it will not break anything.
|
||
|
---
|
||
|
unsquash-4.c | 11 ++++++++---
|
||
|
1 file changed, 8 insertions(+), 3 deletions(-)
|
||
|
|
||
|
diff --git a/squashfs-tools/unsquash-4.c b/squashfs-tools/unsquash-4.c
|
||
|
index ecdaac7..2c0cf63 100644
|
||
|
--- a/squashfs-tools/unsquash-4.c
|
||
|
+++ b/squashfs-tools/unsquash-4.c
|
||
|
@@ -31,9 +31,9 @@ static unsigned int *id_table;
|
||
|
int read_fragment_table_4(long long *directory_table_end)
|
||
|
{
|
||
|
int res, i;
|
||
|
- int bytes = SQUASHFS_FRAGMENT_BYTES(sBlk.s.fragments);
|
||
|
- int indexes = SQUASHFS_FRAGMENT_INDEXES(sBlk.s.fragments);
|
||
|
- long long fragment_table_index[indexes];
|
||
|
+ size_t bytes = SQUASHFS_FRAGMENT_BYTES(sBlk.s.fragments);
|
||
|
+ size_t indexes = SQUASHFS_FRAGMENT_INDEXES(sBlk.s.fragments);
|
||
|
+ long long *fragment_table_index;
|
||
|
|
||
|
TRACE("read_fragment_table: %d fragments, reading %d fragment indexes "
|
||
|
"from 0x%llx\n", sBlk.s.fragments, indexes,
|
||
|
@@ -44,6 +44,11 @@ int read_fragment_table_4(long long *directory_table_end)
|
||
|
return TRUE;
|
||
|
}
|
||
|
|
||
|
+ fragment_table_index = malloc(indexes*sizeof(long long));
|
||
|
+ if(fragment_table_index == NULL)
|
||
|
+ EXIT_UNSQUASH("read_fragment_table: failed to allocate "
|
||
|
+ "fragment table index\n");
|
||
|
+
|
||
|
fragment_table = malloc(bytes);
|
||
|
if(fragment_table == NULL)
|
||
|
EXIT_UNSQUASH("read_fragment_table: failed to allocate "
|