On Thu, Dec 9, 2021 at 2:07 PM Christian Göttsche
<cgzones@xxxxxxxxxxxxxx> wrote:
>
> Check the current item count does not exceed the maximum allowed to
> avoid stack overflows.
>
> ==33660==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fa64b8fc070 at pc 0x0000005acba0 bp 0x7ffc1f0b2870 sp 0x7ffc1f0b2868
> READ of size 4 at 0x7fa64b8fc070 thread T0
> #0 0x5acb9f in avtab_read_item ./libsepol/src/avtab.c:507:18
> #1 0x5acec4 in avtab_read ./libsepol/src/avtab.c:611:8
> #2 0x576ae3 in policydb_read ./libsepol/src/policydb.c:4433:7
> #3 0x55a1fe in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:24:6
> #4 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
> #5 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
> #6 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
> #7 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2)
> #8 0x7fa64cc867ec in __libc_start_main csu/../csu/libc-start.c:332:16
> #9 0x423689 in _start (./out/binpolicy-fuzzer+0x423689)
>
> Address 0x7fa64b8fc070 is located in stack of thread T0 at offset 112 in frame
> #0 0x5aabdf in avtab_read_item ./libsepol/src/avtab.c:437
>
> This frame has 6 object(s):
> [32, 33) 'buf8' (line 438)
> [48, 56) 'buf16' (line 439)
> [80, 112) 'buf32' (line 440) <== Memory access at offset 112 overflows this variable
> [144, 152) 'key' (line 441)
> [176, 192) 'datum' (line 442)
> [208, 244) 'xperms' (line 443)
> HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
> (longjmp and C++ exceptions *are* supported)
> SUMMARY: AddressSanitizer: stack-buffer-overflow ./libsepol/src/avtab.c:507:18 in avtab_read_item
> Shadow bytes around the buggy address:
> 0x0ff5497177b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff5497177c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff5497177d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff5497177e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff5497177f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x0ff549717800: f1 f1 f1 f1 01 f2 00 f2 f2 f2 00 00 00 00[f2]f2
> 0x0ff549717810: f2 f2 00 f2 f2 f2 00 00 f2 f2 00 00 00 00 04 f3
> 0x0ff549717820: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff549717830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff549717840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 0x0ff549717850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==33660==ABORTING
>
> Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> ---
> v3:
> take zero based numbering of variable items into account
> ---
> libsepol/src/avtab.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/libsepol/src/avtab.c b/libsepol/src/avtab.c
> index 46e1e75d..e9b17664 100644
> --- a/libsepol/src/avtab.c
> +++ b/libsepol/src/avtab.c
> @@ -503,6 +503,12 @@ int avtab_read_item(struct policy_file *fp, uint32_t vers, avtab_t * a,
>
> for (i = 0; i < ARRAY_SIZE(spec_order); i++) {
> if (val & spec_order[i]) {
> + if (items >= items2) {
> + ERR(fp->handle,
> + "entry has too many items (%d/%d)",
> + items + /* zero based numbered */ 1, items2);
I do not like comments in the middle of an expression. I will fix this
when I apply the series.
Jim
> + return -1;
> + }
> key.specified = spec_order[i] | enabled;
> datum.data = le32_to_cpu(buf32[items++]);
> rc = insertf(a, &key, &datum, p);
> --
> 2.34.1
>
