| Message ID | 20211105154542.38434-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | libsepol: add fuzzer for reading binary policies | expand |
On Fri, Nov 5, 2021 at 12:11 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > Add a libfuzz[1] based fuzzer testing the reading and parsing of binary policy > files. This fuzzer will be run within the OSS-Fuzz service. > > Handle and reject a variety of edge cases causing crashes or resource leaks. > > The fifth patch ("libsepol/fuzz: limit element sizes for fuzzing") needs some > discussion: To avoid oom reports from the fuzzer, caused by huge memory > allocations, all identifiers are limited to a length of 2^16 for the fuzzer > build only. Probably there should be a limit for the release build too. > Is there a specification for the binary policy format saying something about > the maximum length of identifiers? > After a quick look at the kernel sources (most interesting is str_read()) I > could not find any limits either. > > [1]: https://llvm.org/docs/LibFuzzer.html > > v2: > - reorder patches > 1. oss-fuzz related > 2. libsepol parsing and other crashesand UB > 3. enhance policy validation > - misc changes based on review by James Carter > I have comments on patch 02 and 30, everything else looks good. It was a little hard to work through the reordering, but it does seem like you addressed all my previous comments. Thanks, Jim > Christian Göttsche (36): > cifuzz: enable report-unreproducible-crashes > cifuzz: use the default runtime of 600 seconds > libsepol/fuzz: silence secilc-fuzzer > libsepol: add libfuzz based fuzzer for reading binary policies > libsepol/fuzz: limit element sizes for fuzzing > libsepol: use logging framework in conditional.c > libsepol: use logging framework in ebitmap.c > libsepol: use mallocarray wrapper to avoid overflows > libsepol: use reallocarray wrapper to avoid overflows > libsepol: add checks for read sizes > libsepol: enforce avtab item limit > libsepol: clean memory on conditional insertion failure > libsepol: reject abnormal huge sid ids > libsepol: reject invalid filetrans source type > libsepol: zero member before potential dereference > libsepol: use size_t for indexes in strs helpers > libsepol: do not underflow on short format arguments > libsepol: do not crash on class gaps > libsepol: do not crash on user gaps > libsepol: use correct size for initial string list > libsepol: do not create a string list with initial size zero > libsepol: split validation of datum array gaps and entries > libsepol: validate MLS levels > libsepol: validate expanded user range and level > libsepol: validate permission count of classes > libsepol: resolve log message mismatch > libsepol: validate avtab and avrule types > libsepol: validate constraint expression operators and attributes > libsepol: validate type of avtab type rules > libsepol: validate ocontexts > libsepol: validate genfs contexts > libsepol: validate permissive types > libsepol: validate policy properties > libsepol: validate categories > libsepol: validate fsuse types > libsepol: validate class default targets > > .github/workflows/cifuzz.yml | 3 +- > libsepol/fuzz/binpolicy-fuzzer.c | 63 ++++ > libsepol/fuzz/policy.bin | Bin 0 -> 1552 bytes > libsepol/fuzz/secilc-fuzzer.c | 5 + > libsepol/src/Makefile | 6 + > libsepol/src/avtab.c | 6 + > libsepol/src/conditional.c | 53 ++-- > libsepol/src/ebitmap.c | 27 +- > libsepol/src/expand.c | 4 +- > libsepol/src/hashtab.c | 4 +- > libsepol/src/kernel_to_cil.c | 10 + > libsepol/src/kernel_to_common.c | 23 +- > libsepol/src/kernel_to_common.h | 4 +- > libsepol/src/kernel_to_conf.c | 13 +- > libsepol/src/link.c | 3 +- > libsepol/src/module.c | 4 +- > libsepol/src/module_to_cil.c | 13 +- > libsepol/src/optimize.c | 11 +- > libsepol/src/policydb.c | 27 +- > libsepol/src/policydb_validate.c | 475 +++++++++++++++++++++++++++---- > libsepol/src/private.h | 27 +- > libsepol/src/services.c | 12 +- > libsepol/src/sidtab.c | 3 +- > libsepol/src/user_record.c | 8 +- > libsepol/src/users.c | 12 +- > libsepol/src/util.c | 11 +- > libsepol/src/write.c | 2 +- > scripts/oss-fuzz.sh | 17 +- > 28 files changed, 684 insertions(+), 162 deletions(-) > create mode 100644 libsepol/fuzz/binpolicy-fuzzer.c > create mode 100644 libsepol/fuzz/policy.bin > > -- > 2.33.1 >
On Tue, Nov 9, 2021 at 1:42 PM James Carter <jwcart2@gmail.com> wrote: > > On Fri, Nov 5, 2021 at 12:11 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Add a libfuzz[1] based fuzzer testing the reading and parsing of binary policy > > files. This fuzzer will be run within the OSS-Fuzz service. > > > > Handle and reject a variety of edge cases causing crashes or resource leaks. > > > > The fifth patch ("libsepol/fuzz: limit element sizes for fuzzing") needs some > > discussion: To avoid oom reports from the fuzzer, caused by huge memory > > allocations, all identifiers are limited to a length of 2^16 for the fuzzer > > build only. Probably there should be a limit for the release build too. > > Is there a specification for the binary policy format saying something about > > the maximum length of identifiers? > > After a quick look at the kernel sources (most interesting is str_read()) I > > could not find any limits either. > > > > [1]: https://llvm.org/docs/LibFuzzer.html > > > > v2: > > - reorder patches > > 1. oss-fuzz related > > 2. libsepol parsing and other crashesand UB > > 3. enhance policy validation > > - misc changes based on review by James Carter > > > > I have comments on patch 02 and 30, everything else looks good. It was I meant patches 10 and 30. Jim > a little hard to work through the reordering, but it does seem like > you addressed all my previous comments. > > Thanks, > Jim > > > > Christian Göttsche (36): > > cifuzz: enable report-unreproducible-crashes > > cifuzz: use the default runtime of 600 seconds > > libsepol/fuzz: silence secilc-fuzzer > > libsepol: add libfuzz based fuzzer for reading binary policies > > libsepol/fuzz: limit element sizes for fuzzing > > libsepol: use logging framework in conditional.c > > libsepol: use logging framework in ebitmap.c > > libsepol: use mallocarray wrapper to avoid overflows > > libsepol: use reallocarray wrapper to avoid overflows > > libsepol: add checks for read sizes > > libsepol: enforce avtab item limit > > libsepol: clean memory on conditional insertion failure > > libsepol: reject abnormal huge sid ids > > libsepol: reject invalid filetrans source type > > libsepol: zero member before potential dereference > > libsepol: use size_t for indexes in strs helpers > > libsepol: do not underflow on short format arguments > > libsepol: do not crash on class gaps > > libsepol: do not crash on user gaps > > libsepol: use correct size for initial string list > > libsepol: do not create a string list with initial size zero > > libsepol: split validation of datum array gaps and entries > > libsepol: validate MLS levels > > libsepol: validate expanded user range and level > > libsepol: validate permission count of classes > > libsepol: resolve log message mismatch > > libsepol: validate avtab and avrule types > > libsepol: validate constraint expression operators and attributes > > libsepol: validate type of avtab type rules > > libsepol: validate ocontexts > > libsepol: validate genfs contexts > > libsepol: validate permissive types > > libsepol: validate policy properties > > libsepol: validate categories > > libsepol: validate fsuse types > > libsepol: validate class default targets > > > > .github/workflows/cifuzz.yml | 3 +- > > libsepol/fuzz/binpolicy-fuzzer.c | 63 ++++ > > libsepol/fuzz/policy.bin | Bin 0 -> 1552 bytes > > libsepol/fuzz/secilc-fuzzer.c | 5 + > > libsepol/src/Makefile | 6 + > > libsepol/src/avtab.c | 6 + > > libsepol/src/conditional.c | 53 ++-- > > libsepol/src/ebitmap.c | 27 +- > > libsepol/src/expand.c | 4 +- > > libsepol/src/hashtab.c | 4 +- > > libsepol/src/kernel_to_cil.c | 10 + > > libsepol/src/kernel_to_common.c | 23 +- > > libsepol/src/kernel_to_common.h | 4 +- > > libsepol/src/kernel_to_conf.c | 13 +- > > libsepol/src/link.c | 3 +- > > libsepol/src/module.c | 4 +- > > libsepol/src/module_to_cil.c | 13 +- > > libsepol/src/optimize.c | 11 +- > > libsepol/src/policydb.c | 27 +- > > libsepol/src/policydb_validate.c | 475 +++++++++++++++++++++++++++---- > > libsepol/src/private.h | 27 +- > > libsepol/src/services.c | 12 +- > > libsepol/src/sidtab.c | 3 +- > > libsepol/src/user_record.c | 8 +- > > libsepol/src/users.c | 12 +- > > libsepol/src/util.c | 11 +- > > libsepol/src/write.c | 2 +- > > scripts/oss-fuzz.sh | 17 +- > > 28 files changed, 684 insertions(+), 162 deletions(-) > > create mode 100644 libsepol/fuzz/binpolicy-fuzzer.c > > create mode 100644 libsepol/fuzz/policy.bin > > > > -- > > 2.33.1 > >
Add a libfuzz[1] based fuzzer testing the reading and parsing of binary policy files. This fuzzer will be run within the OSS-Fuzz service. Handle and reject a variety of edge cases causing crashes or resource leaks. The fifth patch ("libsepol/fuzz: limit element sizes for fuzzing") needs some discussion: To avoid oom reports from the fuzzer, caused by huge memory allocations, all identifiers are limited to a length of 2^16 for the fuzzer build only. Probably there should be a limit for the release build too. Is there a specification for the binary policy format saying something about the maximum length of identifiers? After a quick look at the kernel sources (most interesting is str_read()) I could not find any limits either. [1]: https://llvm.org/docs/LibFuzzer.html v2: - reorder patches 1. oss-fuzz related 2. libsepol parsing and other crashesand UB 3. enhance policy validation - misc changes based on review by James Carter Christian Göttsche (36): cifuzz: enable report-unreproducible-crashes cifuzz: use the default runtime of 600 seconds libsepol/fuzz: silence secilc-fuzzer libsepol: add libfuzz based fuzzer for reading binary policies libsepol/fuzz: limit element sizes for fuzzing libsepol: use logging framework in conditional.c libsepol: use logging framework in ebitmap.c libsepol: use mallocarray wrapper to avoid overflows libsepol: use reallocarray wrapper to avoid overflows libsepol: add checks for read sizes libsepol: enforce avtab item limit libsepol: clean memory on conditional insertion failure libsepol: reject abnormal huge sid ids libsepol: reject invalid filetrans source type libsepol: zero member before potential dereference libsepol: use size_t for indexes in strs helpers libsepol: do not underflow on short format arguments libsepol: do not crash on class gaps libsepol: do not crash on user gaps libsepol: use correct size for initial string list libsepol: do not create a string list with initial size zero libsepol: split validation of datum array gaps and entries libsepol: validate MLS levels libsepol: validate expanded user range and level libsepol: validate permission count of classes libsepol: resolve log message mismatch libsepol: validate avtab and avrule types libsepol: validate constraint expression operators and attributes libsepol: validate type of avtab type rules libsepol: validate ocontexts libsepol: validate genfs contexts libsepol: validate permissive types libsepol: validate policy properties libsepol: validate categories libsepol: validate fsuse types libsepol: validate class default targets .github/workflows/cifuzz.yml | 3 +- libsepol/fuzz/binpolicy-fuzzer.c | 63 ++++ libsepol/fuzz/policy.bin | Bin 0 -> 1552 bytes libsepol/fuzz/secilc-fuzzer.c | 5 + libsepol/src/Makefile | 6 + libsepol/src/avtab.c | 6 + libsepol/src/conditional.c | 53 ++-- libsepol/src/ebitmap.c | 27 +- libsepol/src/expand.c | 4 +- libsepol/src/hashtab.c | 4 +- libsepol/src/kernel_to_cil.c | 10 + libsepol/src/kernel_to_common.c | 23 +- libsepol/src/kernel_to_common.h | 4 +- libsepol/src/kernel_to_conf.c | 13 +- libsepol/src/link.c | 3 +- libsepol/src/module.c | 4 +- libsepol/src/module_to_cil.c | 13 +- libsepol/src/optimize.c | 11 +- libsepol/src/policydb.c | 27 +- libsepol/src/policydb_validate.c | 475 +++++++++++++++++++++++++++---- libsepol/src/private.h | 27 +- libsepol/src/services.c | 12 +- libsepol/src/sidtab.c | 3 +- libsepol/src/user_record.c | 8 +- libsepol/src/users.c | 12 +- libsepol/src/util.c | 11 +- libsepol/src/write.c | 2 +- scripts/oss-fuzz.sh | 17 +- 28 files changed, 684 insertions(+), 162 deletions(-) create mode 100644 libsepol/fuzz/binpolicy-fuzzer.c create mode 100644 libsepol/fuzz/policy.bin