| Message ID | 20211105154542.38434-11-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Superseded |
| 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 checks for invalid read sizes from a binary policy to guard > allocations. > > The common and class permission counts needs to be limited more strict > otherwise a too high count of common or class permissions can lead to > permission values with a too high value, which can lead to overflows > in shift operations. > > In the fuzzer build the value will also be bounded to avoid oom reports. > > ==29857== ERROR: libFuzzer: out-of-memory (malloc(17179868160)) > To change the out-of-memory limit use -rss_limit_mb=<N> > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > #6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143) > #7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb) > #8 0x580b5d in mallocarray ./libsepol/src/./private.h:93:9 > #9 0x57c2ed in scope_read ./libsepol/src/policydb.c:4120:7 > #10 0x576b0d in policydb_read ./libsepol/src/policydb.c:4462:9 > #11 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > #12 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > #13 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > #14 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > #15 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > #16 0x7ffad6e107ec in __libc_start_main csu/../csu/libc-start.c:332:16 > #17 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > ==19462== ERROR: libFuzzer: out-of-memory (malloc(18253611008)) > To change the out-of-memory limit use -rss_limit_mb=<N> > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > #6 0x4aa999 in __asan::asan_calloc(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa999) > #7 0x525b63 in __interceptor_calloc (./out/binpolicy-fuzzer+0x525b63) > #8 0x570938 in policydb_index_others ./libsepol/src/policydb.c:1245:6 > #9 0x5771f3 in policydb_read ./src/policydb.c:4481:6 > #10 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > #11 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > #12 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > #13 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > #14 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > #15 0x7f4d933157ec in __libc_start_main csu/../csu/libc-start.c:332:16 > #16 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > libsepol/src/policydb.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > index dcea1807..1408405d 100644 > --- a/libsepol/src/policydb.c > +++ b/libsepol/src/policydb.c > @@ -2103,6 +2103,8 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > if (symtab_init(&comdatum->permissions, PERM_SYMTAB_SIZE)) > goto bad; > comdatum->permissions.nprim = le32_to_cpu(buf[2]); > + if (comdatum->permissions.nprim > 32) Should use PERM_SYMTAB_SIZE here (like in patch 22). > + goto bad; > nel = le32_to_cpu(buf[3]); > > key = malloc(len + 1); > @@ -2251,6 +2253,8 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) > goto bad; > cladatum->permissions.nprim = le32_to_cpu(buf[3]); > + if (cladatum->permissions.nprim > 32) Same here. > + goto bad; > nel = le32_to_cpu(buf[4]); > > ncons = le32_to_cpu(buf[5]); > @@ -3980,6 +3984,8 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl, > if (rc < 0) > return -1; > nprim = le32_to_cpu(buf[0]); > + if (is_saturated(nprim)) > + return -1; > nel = le32_to_cpu(buf[1]); > for (j = 0; j < nel; j++) { > if (read_f[i] (p, decl->symtab[i].table, fp)) { > @@ -4106,7 +4112,7 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) > goto cleanup; > scope->scope = le32_to_cpu(buf[0]); > scope->decl_ids_len = le32_to_cpu(buf[1]); > - if (scope->decl_ids_len == 0) { > + if (zero_or_saturated(scope->decl_ids_len)) { > ERR(fp->handle, "invalid scope with no declaration"); > goto cleanup; > } > @@ -4396,6 +4402,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) > if (rc < 0) > goto bad; > nprim = le32_to_cpu(buf[0]); > + if (is_saturated(nprim)) > + goto bad; > nel = le32_to_cpu(buf[1]); > if (nel && !nprim) { > ERR(fp->handle, "unexpected items in symbol table with no symbol"); > -- > 2.33.1 >
On Tue, 9 Nov 2021 at 19:46, James Carter <jwcart2@gmail.com> wrote: > > On Fri, Nov 5, 2021 at 12:11 PM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Add checks for invalid read sizes from a binary policy to guard > > allocations. > > > > The common and class permission counts needs to be limited more strict > > otherwise a too high count of common or class permissions can lead to > > permission values with a too high value, which can lead to overflows > > in shift operations. > > > > In the fuzzer build the value will also be bounded to avoid oom reports. > > > > ==29857== ERROR: libFuzzer: out-of-memory (malloc(17179868160)) > > To change the out-of-memory limit use -rss_limit_mb=<N> > > > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > > #6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143) > > #7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb) > > #8 0x580b5d in mallocarray ./libsepol/src/./private.h:93:9 > > #9 0x57c2ed in scope_read ./libsepol/src/policydb.c:4120:7 > > #10 0x576b0d in policydb_read ./libsepol/src/policydb.c:4462:9 > > #11 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > > #12 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > > #13 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > > #14 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > > #15 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > > #16 0x7ffad6e107ec in __libc_start_main csu/../csu/libc-start.c:332:16 > > #17 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > > > ==19462== ERROR: libFuzzer: out-of-memory (malloc(18253611008)) > > To change the out-of-memory limit use -rss_limit_mb=<N> > > > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > > #6 0x4aa999 in __asan::asan_calloc(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa999) > > #7 0x525b63 in __interceptor_calloc (./out/binpolicy-fuzzer+0x525b63) > > #8 0x570938 in policydb_index_others ./libsepol/src/policydb.c:1245:6 > > #9 0x5771f3 in policydb_read ./src/policydb.c:4481:6 > > #10 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > > #11 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > > #12 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > > #13 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > > #14 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > > #15 0x7f4d933157ec in __libc_start_main csu/../csu/libc-start.c:332:16 > > #16 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > libsepol/src/policydb.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > index dcea1807..1408405d 100644 > > --- a/libsepol/src/policydb.c > > +++ b/libsepol/src/policydb.c > > @@ -2103,6 +2103,8 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > > if (symtab_init(&comdatum->permissions, PERM_SYMTAB_SIZE)) > > goto bad; > > comdatum->permissions.nprim = le32_to_cpu(buf[2]); > > + if (comdatum->permissions.nprim > 32) > > Should use PERM_SYMTAB_SIZE here (like in patch 22). I thought about that, but it seemed a bit unrelated. In patch 25 (https://patchwork.kernel.org/project/selinux/patch/20211105154542.38434-26-cgzones@googlemail.com/) the number of permissions in commons and classes is checked against the logical limit (which is imposed by the implementation via a bitset on a 32 bit integer). The check here is to prevent overflows on subsequent left-shifts by the checked value on a 32 bit integer. Maybe `sizeof(uint32_t)` is more expressive? However I do not object your comment, so I fine to change to PERM_SYMTAB_SIZE. > > + goto bad; > > nel = le32_to_cpu(buf[3]); > > > > key = malloc(len + 1); > > @@ -2251,6 +2253,8 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > > if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) > > goto bad; > > cladatum->permissions.nprim = le32_to_cpu(buf[3]); > > + if (cladatum->permissions.nprim > 32) > > Same here. > > > > + goto bad; > > nel = le32_to_cpu(buf[4]); > > > > ncons = le32_to_cpu(buf[5]); > > @@ -3980,6 +3984,8 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl, > > if (rc < 0) > > return -1; > > nprim = le32_to_cpu(buf[0]); > > + if (is_saturated(nprim)) > > + return -1; > > nel = le32_to_cpu(buf[1]); > > for (j = 0; j < nel; j++) { > > if (read_f[i] (p, decl->symtab[i].table, fp)) { > > @@ -4106,7 +4112,7 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) > > goto cleanup; > > scope->scope = le32_to_cpu(buf[0]); > > scope->decl_ids_len = le32_to_cpu(buf[1]); > > - if (scope->decl_ids_len == 0) { > > + if (zero_or_saturated(scope->decl_ids_len)) { > > ERR(fp->handle, "invalid scope with no declaration"); > > goto cleanup; > > } > > @@ -4396,6 +4402,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) > > if (rc < 0) > > goto bad; > > nprim = le32_to_cpu(buf[0]); > > + if (is_saturated(nprim)) > > + goto bad; > > nel = le32_to_cpu(buf[1]); > > if (nel && !nprim) { > > ERR(fp->handle, "unexpected items in symbol table with no symbol"); > > -- > > 2.33.1 > >
On Tue, Nov 9, 2021 at 1:59 PM Christian Göttsche <cgzones@googlemail.com> wrote: > > On Tue, 9 Nov 2021 at 19:46, James Carter <jwcart2@gmail.com> wrote: > > > > On Fri, Nov 5, 2021 at 12:11 PM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > > > > Add checks for invalid read sizes from a binary policy to guard > > > allocations. > > > > > > The common and class permission counts needs to be limited more strict > > > otherwise a too high count of common or class permissions can lead to > > > permission values with a too high value, which can lead to overflows > > > in shift operations. > > > > > > In the fuzzer build the value will also be bounded to avoid oom reports. > > > > > > ==29857== ERROR: libFuzzer: out-of-memory (malloc(17179868160)) > > > To change the out-of-memory limit use -rss_limit_mb=<N> > > > > > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > > > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > > > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > > > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > > > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > > > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > > > #6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143) > > > #7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb) > > > #8 0x580b5d in mallocarray ./libsepol/src/./private.h:93:9 > > > #9 0x57c2ed in scope_read ./libsepol/src/policydb.c:4120:7 > > > #10 0x576b0d in policydb_read ./libsepol/src/policydb.c:4462:9 > > > #11 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > > > #12 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > > > #13 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > > > #14 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > > > #15 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > > > #16 0x7ffad6e107ec in __libc_start_main csu/../csu/libc-start.c:332:16 > > > #17 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > > > > > ==19462== ERROR: libFuzzer: out-of-memory (malloc(18253611008)) > > > To change the out-of-memory limit use -rss_limit_mb=<N> > > > > > > #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) > > > #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o > > > #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o > > > #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o > > > #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) > > > #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) > > > #6 0x4aa999 in __asan::asan_calloc(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa999) > > > #7 0x525b63 in __interceptor_calloc (./out/binpolicy-fuzzer+0x525b63) > > > #8 0x570938 in policydb_index_others ./libsepol/src/policydb.c:1245:6 > > > #9 0x5771f3 in policydb_read ./src/policydb.c:4481:6 > > > #10 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 > > > #11 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o > > > #12 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o > > > #13 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o > > > #14 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) > > > #15 0x7f4d933157ec in __libc_start_main csu/../csu/libc-start.c:332:16 > > > #16 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) > > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > > --- > > > libsepol/src/policydb.c | 10 +++++++++- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > > index dcea1807..1408405d 100644 > > > --- a/libsepol/src/policydb.c > > > +++ b/libsepol/src/policydb.c > > > @@ -2103,6 +2103,8 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > > > if (symtab_init(&comdatum->permissions, PERM_SYMTAB_SIZE)) > > > goto bad; > > > comdatum->permissions.nprim = le32_to_cpu(buf[2]); > > > + if (comdatum->permissions.nprim > 32) > > > > Should use PERM_SYMTAB_SIZE here (like in patch 22). > > I thought about that, but it seemed a bit unrelated. > In patch 25 (https://patchwork.kernel.org/project/selinux/patch/20211105154542.38434-26-cgzones@googlemail.com/) > the number of permissions in commons and classes is checked against > the logical limit (which is imposed by the implementation via a bitset > on a 32 bit integer). > The check here is to prevent overflows on subsequent left-shifts by > the checked value on a 32 bit integer. > Maybe `sizeof(uint32_t)` is more expressive? > However I do not object your comment, so I fine to change to PERM_SYMTAB_SIZE. > I think that you make a good point, but I think that PERM_SYMTAB_SIZE, while not perfect, is the best choice. Thanks, Jim > > > + goto bad; > > > nel = le32_to_cpu(buf[3]); > > > > > > key = malloc(len + 1); > > > @@ -2251,6 +2253,8 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) > > > if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) > > > goto bad; > > > cladatum->permissions.nprim = le32_to_cpu(buf[3]); > > > + if (cladatum->permissions.nprim > 32) > > > > Same here. > > > > > > > + goto bad; > > > nel = le32_to_cpu(buf[4]); > > > > > > ncons = le32_to_cpu(buf[5]); > > > @@ -3980,6 +3984,8 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl, > > > if (rc < 0) > > > return -1; > > > nprim = le32_to_cpu(buf[0]); > > > + if (is_saturated(nprim)) > > > + return -1; > > > nel = le32_to_cpu(buf[1]); > > > for (j = 0; j < nel; j++) { > > > if (read_f[i] (p, decl->symtab[i].table, fp)) { > > > @@ -4106,7 +4112,7 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) > > > goto cleanup; > > > scope->scope = le32_to_cpu(buf[0]); > > > scope->decl_ids_len = le32_to_cpu(buf[1]); > > > - if (scope->decl_ids_len == 0) { > > > + if (zero_or_saturated(scope->decl_ids_len)) { > > > ERR(fp->handle, "invalid scope with no declaration"); > > > goto cleanup; > > > } > > > @@ -4396,6 +4402,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) > > > if (rc < 0) > > > goto bad; > > > nprim = le32_to_cpu(buf[0]); > > > + if (is_saturated(nprim)) > > > + goto bad; > > > nel = le32_to_cpu(buf[1]); > > > if (nel && !nprim) { > > > ERR(fp->handle, "unexpected items in symbol table with no symbol"); > > > -- > > > 2.33.1 > > >
diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index dcea1807..1408405d 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -2103,6 +2103,8 @@ static int common_read(policydb_t * p, hashtab_t h, struct policy_file *fp) if (symtab_init(&comdatum->permissions, PERM_SYMTAB_SIZE)) goto bad; comdatum->permissions.nprim = le32_to_cpu(buf[2]); + if (comdatum->permissions.nprim > 32) + goto bad; nel = le32_to_cpu(buf[3]); key = malloc(len + 1); @@ -2251,6 +2253,8 @@ static int class_read(policydb_t * p, hashtab_t h, struct policy_file *fp) if (symtab_init(&cladatum->permissions, PERM_SYMTAB_SIZE)) goto bad; cladatum->permissions.nprim = le32_to_cpu(buf[3]); + if (cladatum->permissions.nprim > 32) + goto bad; nel = le32_to_cpu(buf[4]); ncons = le32_to_cpu(buf[5]); @@ -3980,6 +3984,8 @@ static int avrule_decl_read(policydb_t * p, avrule_decl_t * decl, if (rc < 0) return -1; nprim = le32_to_cpu(buf[0]); + if (is_saturated(nprim)) + return -1; nel = le32_to_cpu(buf[1]); for (j = 0; j < nel; j++) { if (read_f[i] (p, decl->symtab[i].table, fp)) { @@ -4106,7 +4112,7 @@ static int scope_read(policydb_t * p, int symnum, struct policy_file *fp) goto cleanup; scope->scope = le32_to_cpu(buf[0]); scope->decl_ids_len = le32_to_cpu(buf[1]); - if (scope->decl_ids_len == 0) { + if (zero_or_saturated(scope->decl_ids_len)) { ERR(fp->handle, "invalid scope with no declaration"); goto cleanup; } @@ -4396,6 +4402,8 @@ int policydb_read(policydb_t * p, struct policy_file *fp, unsigned verbose) if (rc < 0) goto bad; nprim = le32_to_cpu(buf[0]); + if (is_saturated(nprim)) + goto bad; nel = le32_to_cpu(buf[1]); if (nel && !nprim) { ERR(fp->handle, "unexpected items in symbol table with no symbol");
Add checks for invalid read sizes from a binary policy to guard allocations. The common and class permission counts needs to be limited more strict otherwise a too high count of common or class permissions can lead to permission values with a too high value, which can lead to overflows in shift operations. In the fuzzer build the value will also be bounded to avoid oom reports. ==29857== ERROR: libFuzzer: out-of-memory (malloc(17179868160)) To change the out-of-memory limit use -rss_limit_mb=<N> #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) #6 0x4aa143 in __asan::asan_malloc(unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa143) #7 0x5259cb in malloc (./out/binpolicy-fuzzer+0x5259cb) #8 0x580b5d in mallocarray ./libsepol/src/./private.h:93:9 #9 0x57c2ed in scope_read ./libsepol/src/policydb.c:4120:7 #10 0x576b0d in policydb_read ./libsepol/src/policydb.c:4462:9 #11 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 #12 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o #13 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o #14 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o #15 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) #16 0x7ffad6e107ec in __libc_start_main csu/../csu/libc-start.c:332:16 #17 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) ==19462== ERROR: libFuzzer: out-of-memory (malloc(18253611008)) To change the out-of-memory limit use -rss_limit_mb=<N> #0 0x52dc61 in __sanitizer_print_stack_trace (./out/binpolicy-fuzzer+0x52dc61) #1 0x475618 in fuzzer::PrintStackTrace() fuzzer.o #2 0x458855 in fuzzer::Fuzzer::HandleMalloc(unsigned long) fuzzer.o #3 0x45876a in fuzzer::MallocHook(void const volatile*, unsigned long) fuzzer.o #4 0x534557 in __sanitizer::RunMallocHooks(void const*, unsigned long) (./out/binpolicy-fuzzer+0x534557) #5 0x4aa7d7 in __asan::Allocator::Allocate(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, bool) (./out/binpolicy-fuzzer+0x4aa7d7) #6 0x4aa999 in __asan::asan_calloc(unsigned long, unsigned long, __sanitizer::BufferedStackTrace*) (./out/binpolicy-fuzzer+0x4aa999) #7 0x525b63 in __interceptor_calloc (./out/binpolicy-fuzzer+0x525b63) #8 0x570938 in policydb_index_others ./libsepol/src/policydb.c:1245:6 #9 0x5771f3 in policydb_read ./src/policydb.c:4481:6 #10 0x55a214 in LLVMFuzzerTestOneInput ./libsepol/fuzz/binpolicy-fuzzer.c:26:6 #11 0x45aed3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o #12 0x446a12 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o #13 0x44c93b in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o #14 0x475dd2 in main (./out/binpolicy-fuzzer+0x475dd2) #15 0x7f4d933157ec in __libc_start_main csu/../csu/libc-start.c:332:16 #16 0x423689 in _start (./out/binpolicy-fuzzer+0x423689) Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- libsepol/src/policydb.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)