1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
|
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 "
|
