| Message ID | 20211209164928.87459-12-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libsepol: add fuzzer for reading binary policies | expand |
On Thu, Dec 9, 2021 at 2:07 PM Christian Göttsche <cgzones@googlemail.com> 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@googlemail.com> > --- > 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 >
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); + return -1; + } key.specified = spec_order[i] | enabled; datum.data = le32_to_cpu(buf32[items++]); rc = insertf(a, &key, &datum, p);
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@googlemail.com> --- v3: take zero based numbering of variable items into account --- libsepol/src/avtab.c | 6 ++++++ 1 file changed, 6 insertions(+)