From 5f76354ceda3d2fce17b4b57e289194c8ef8a092 Mon Sep 17 00:00:00 2001 From: yang_zhuang_zhuang <1162011203@qq.com> Date: Mon, 15 Mar 2021 11:47:00 +0800 Subject: [PATCH] fix heap-use-after-free in cil_yy_switch_to_buffer fix heap-use-after-free in __class_reset_perm_values() fix heap-buffer-overflow in cil_print_recursive_blockinherit --- ...ix-heap-use-after-free-in_class-rese.patch | 90 ++++++++++++++ ...l-cil-always-destroy-the-lexer-state.patch | 112 ++++++++++++++++++ ...stroy-perm_datums-when-_cil_resolve_.patch | 36 ++++++ ...o-not-add-a-stack-variable-to-a-list.patch | 39 ++++++ ...-NULL-pointer-dereference-when-parsi.patch | 82 +++++++++++++ ...-NULL-pointer-dereference-when-using.patch | 41 +++++++ ...-out-of-bound-read-in-cil_print_recu.patch | 57 +++++++++ ...l-propagate-failure-of-cil_fill_list.patch | 49 ++++++++ libsepol.spec | 15 ++- 9 files changed, 520 insertions(+), 1 deletion(-) create mode 100644 backport-libsepol-cil-Fix-heap-use-after-free-in_class-rese.patch create mode 100644 backport-libsepol-cil-always-destroy-the-lexer-state.patch create mode 100644 backport-libsepol-cil-destroy-perm_datums-when-_cil_resolve_.patch create mode 100644 backport-libsepol-cil-do-not-add-a-stack-variable-to-a-list.patch create mode 100644 backport-libsepol-cil-fix-NULL-pointer-dereference-when-parsi.patch create mode 100644 backport-libsepol-cil-fix-NULL-pointer-dereference-when-using.patch create mode 100644 backport-libsepol-cil-fix-out-of-bound-read-in-cil_print_recu.patch create mode 100644 backport-libsepol-cil-propagate-failure-of-cil_fill_list.patch diff --git a/backport-libsepol-cil-Fix-heap-use-after-free-in_class-rese.patch b/backport-libsepol-cil-Fix-heap-use-after-free-in_class-rese.patch new file mode 100644 index 0000000..f32ce6e --- /dev/null +++ b/backport-libsepol-cil-Fix-heap-use-after-free-in_class-rese.patch @@ -0,0 +1,90 @@ +From f0d98f83d28a0cdf437b4cafe229e27cb91eb493 Mon Sep 17 00:00:00 2001 +From: James Carter +Date: Wed, 6 Jan 2021 13:43:26 -0500 +Subject: [PATCH] libsepol/cil: Fix heap-use-after-free in + __class_reset_perm_values() + +Nicolas Iooss reports: + A few weeks ago, OSS-Fuzz got configured in order to fuzz the CIL + policy compiler (cf. + https://github.com/SELinuxProject/selinux/issues/215 and + https://github.com/google/oss-fuzz/pull/4790). It reported a bunch of + simple issues, for which I will submit patches. There are also more + subtle bugs, like the one triggered by this CIL policy: + + (class CLASS (PERM)) + (classorder (CLASS)) + (sid SID) + (sidorder (SID)) + (sensitivity SENS) + (sensitivityorder (SENS)) + (type TYPE) + (allow TYPE self (CLASS (PERM))) + + (block b + (optional o + (sensitivitycategory SENS (C)) ; Non-existing category + triggers disabling the optional + (common COMMON (PERM1)) + (classcommon CLASS COMMON) + (allow TYPE self (CLASS (PERM1))) + ) + ) + + On my computer, secilc manages to build this policy fine, but when + clang's Address Sanitizer is enabled, running secilc leads to the + following report: + + $ make -C libsepol/src CC=clang CFLAGS='-g -fsanitize=address' libsepol.a + $ clang -g -fsanitize=address secilc/secilc.c libsepol/src/libsepol.a + -o my_secilc + $ ./my_secilc -vv testcase.cil + Parsing testcase.cil + Building AST from Parse Tree + Destroying Parse Tree + Resolving AST + Failed to resolve sensitivitycategory statement at testcase.cil:12 + Disabling optional 'o' at testcase.cil:11 + Resetting declarations + ================================================================= + ==181743==ERROR: AddressSanitizer: heap-use-after-free on address + 0x6070000000c0 at pc 0x55ff7e445d24 bp 0x7ffe7eecfba0 sp + 0x7ffe7eecfb98 + READ of size 4 at 0x6070000000c0 thread T0 + #0 0x55ff7e445d23 in __class_reset_perm_values + /git/selinux-userspace/libsepol/src/../cil/src/cil_reset_ast.c:17:17 + +The problem is that the optional branch is destroyed when it is disabled, +so the common has already been destroyed when the reset code tries to +access the number of common permissions, so that it can change the +value of the class permissions back to their original values. + +The solution is to count the number of class permissions and then +calculate the number of common permissions. + +Reported-by: Nicolas Iooss +Signed-off-by: James Carter +--- + libsepol/cil/src/cil_reset_ast.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/libsepol/cil/src/cil_reset_ast.c b/libsepol/cil/src/cil_reset_ast.c +index 43e6b88e0..52e5f6401 100644 +--- a/libsepol/cil/src/cil_reset_ast.c ++++ b/libsepol/cil/src/cil_reset_ast.c +@@ -22,11 +22,12 @@ static int __class_reset_perm_values(__attribute__((unused)) hashtab_key_t k, ha + static void cil_reset_class(struct cil_class *class) + { + if (class->common != NULL) { +- struct cil_class *common = class->common; +- cil_symtab_map(&class->perms, __class_reset_perm_values, &common->num_perms); ++ /* Must assume that the common has been destroyed */ ++ int num_common_perms = class->num_perms - class->perms.nprim; ++ cil_symtab_map(&class->perms, __class_reset_perm_values, &num_common_perms); + /* during a re-resolve, we need to reset the common, so a classcommon + * statement isn't seen as a duplicate */ +- class->num_perms -= common->num_perms; ++ class->num_perms = class->perms.nprim; + class->common = NULL; /* Must make this NULL or there will be an error when re-resolving */ + } + class->ordered = CIL_FALSE; diff --git a/backport-libsepol-cil-always-destroy-the-lexer-state.patch b/backport-libsepol-cil-always-destroy-the-lexer-state.patch new file mode 100644 index 0000000..76bb5ef --- /dev/null +++ b/backport-libsepol-cil-always-destroy-the-lexer-state.patch @@ -0,0 +1,112 @@ +From 90809674c13c03e324dff560654d0e686c5fc46b Mon Sep 17 00:00:00 2001 +From: Evgeny Vereshchagin +Date: Sun, 6 Dec 2020 23:29:22 +0100 +Subject: [PATCH] libsepol/cil: always destroy the lexer state + +It was found in https://github.com/google/oss-fuzz/pull/4790: +``` +Invalid token '' at line 2 of fuzz + NEW_FUNC[1/2]: 0x67fff0 in yy_get_previous_state /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1143 + NEW_FUNC[2/2]: 0x6803e0 in yy_try_NUL_trans /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1176 +================================================================= +==12==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000007992 at pc 0x000000681800 bp 0x7ffccddee530 sp 0x7ffccddee528 +WRITE of size 1 at 0x602000007992 thread T0 +SCARINESS: 41 (1-byte-write-heap-use-after-free) + #0 0x6817ff in cil_yy_switch_to_buffer /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1315:17 + #1 0x6820cc in cil_yy_scan_buffer /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1571:2 + #2 0x682662 in cil_lexer_setup /src/selinux/libsepol/src/../cil/src/cil_lexer.l:73:6 + #3 0x5cf2ae in cil_parser /src/selinux/libsepol/src/../cil/src/cil_parser.c:220:2 + #4 0x56d5e2 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:514:7 + #5 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7 + #6 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 + #7 0x45a755 in fuzzer::Fuzzer::TryDetectingAMemoryLeak(unsigned char const*, unsigned long, bool) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:675:3 + #8 0x45acd9 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:747:5 + #9 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5 + #10 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6 + #11 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 + #12 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f) + #13 0x41e758 in _start (/out/secilc-fuzzer+0x41e758) + +DEDUP_TOKEN: cil_yy_switch_to_buffer--cil_yy_scan_buffer--cil_lexer_setup +0x602000007992 is located 2 bytes inside of 4-byte region [0x602000007990,0x602000007994) +freed by thread T0 here: + #0 0x521ef2 in free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:127:3 + #1 0x56d630 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:526:2 + #2 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7 + #3 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 + #4 0x458fba in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3 + #5 0x45acc7 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19 + #6 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5 + #7 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6 + #8 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 + #9 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f) + +DEDUP_TOKEN: free--cil_add_file--LLVMFuzzerTestOneInput +previously allocated by thread T0 here: + #0 0x52215d in malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 + #1 0x5cecb8 in cil_malloc /src/selinux/libsepol/src/../cil/src/cil_mem.c:39:14 + #2 0x56d584 in cil_add_file /src/selinux/libsepol/src/../cil/src/cil.c:510:11 + #3 0x556e91 in LLVMFuzzerTestOneInput /src/secilc-fuzzer.c:434:7 + #4 0x459ab1 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15 + #5 0x458fba in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3 + #6 0x45acc7 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19 + #7 0x45b875 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5 + #8 0x4499fb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6 + #9 0x473a32 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 + #10 0x7f982296d83f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f) + +DEDUP_TOKEN: malloc--cil_malloc--cil_add_file +SUMMARY: AddressSanitizer: heap-use-after-free /src/selinux/libsepol/src/../cil/src/cil_lexer.c:1315:17 in cil_yy_switch_to_buffer +Shadow bytes around the buggy address: + 0x0c047fff8ee0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd + 0x0c047fff8ef0: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd + 0x0c047fff8f00: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd + 0x0c047fff8f10: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd + 0x0c047fff8f20: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa +=>0x0c047fff8f30: fa fa[fd]fa fa fa fd fa fa fa fd fa fa fa fd fa + 0x0c047fff8f40: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa + 0x0c047fff8f50: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa + 0x0c047fff8f60: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fa + 0x0c047fff8f70: fa fa 00 00 fa fa 02 fa fa fa 02 fa fa fa 00 fa + 0x0c047fff8f80: fa fa 03 fa fa fa 00 fa fa fa 03 fa fa fa 00 fa +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 + Shadow gap: cc +==12==ABORTING +``` + +Signed-off-by: Evgeny Vereshchagin +Acked-by: Nicolas Iooss +--- + libsepol/cil/src/cil_parser.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c +index 585ea7704..a8af1dce2 100644 +--- a/libsepol/cil/src/cil_parser.c ++++ b/libsepol/cil/src/cil_parser.c +@@ -310,6 +310,7 @@ int cil_parser(char *_path, char *buffer, uint32_t size, struct cil_tree **parse + while (!cil_stack_is_empty(stack)) { + pop_hll_info(stack, &hll_lineno, &hll_expand); + } ++ cil_lexer_destroy(); + cil_stack_destroy(&stack); + + return SEPOL_ERR; diff --git a/backport-libsepol-cil-destroy-perm_datums-when-_cil_resolve_.patch b/backport-libsepol-cil-destroy-perm_datums-when-_cil_resolve_.patch new file mode 100644 index 0000000..657be19 --- /dev/null +++ b/backport-libsepol-cil-destroy-perm_datums-when-_cil_resolve_.patch @@ -0,0 +1,36 @@ +From b7ea65f547c67bfbae4ae133052583b090747e5a Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 30 Dec 2020 11:07:46 +0100 +Subject: [PATCH] libsepol/cil: destroy perm_datums when __cil_resolve_perms + fails + +When __cil_resolve_perms fails, it does not destroy perm_datums, which +leads to a memory leak reported by OSS-Fuzz with the following CIL +policy: + + (class cl01()) + (classorder(cl01)) + (type at02) + (type tpr3) + (allow at02 tpr3(cl01((s)))) + +Calling cil_list_destroy() fixes the issue. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28466 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_resolve_ast.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index 68b590bcc..0c85eabe5 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -146,6 +146,7 @@ static int __cil_resolve_perms(symtab_t *class_symtab, symtab_t *common_symtab, + return SEPOL_OK; + + exit: ++ cil_list_destroy(perm_datums, CIL_FALSE); + return rc; + } + diff --git a/backport-libsepol-cil-do-not-add-a-stack-variable-to-a-list.patch b/backport-libsepol-cil-do-not-add-a-stack-variable-to-a-list.patch new file mode 100644 index 0000000..079cc11 --- /dev/null +++ b/backport-libsepol-cil-do-not-add-a-stack-variable-to-a-list.patch @@ -0,0 +1,39 @@ +From 6c8fca10452e445edf52daffc69f2e9dfcdac54c Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 30 Dec 2020 21:11:40 +0100 +Subject: [PATCH] libsepol/cil: do not add a stack variable to a list + +OSS-Fuzz found a heap use-after-free when the CIL compiler destroys its +database after failing to compile the following policy: + + (validatetrans x (eq t3 (a))) + +This is caused by the fact that the validatetrans AST object references +a stack variable local to __cil_fill_constraint_leaf_expr, when parsing +the list "(a)": + + struct cil_list *sub_list; + cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); + cil_list_append(*leaf_expr, CIL_LIST, &sub_list); + +Drop the & sign to really add the list like it is supposed to be. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28507 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_build_ast.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index 67801def0..d5b9977c0 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -2714,7 +2714,7 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c + } else if (r_flavor == CIL_LIST) { + struct cil_list *sub_list; + cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); +- cil_list_append(*leaf_expr, CIL_LIST, &sub_list); ++ cil_list_append(*leaf_expr, CIL_LIST, sub_list); + } else { + cil_list_append(*leaf_expr, CIL_CONS_OPERAND, (void *)r_flavor); + } diff --git a/backport-libsepol-cil-fix-NULL-pointer-dereference-when-parsi.patch b/backport-libsepol-cil-fix-NULL-pointer-dereference-when-parsi.patch new file mode 100644 index 0000000..7b475e8 --- /dev/null +++ b/backport-libsepol-cil-fix-NULL-pointer-dereference-when-parsi.patch @@ -0,0 +1,82 @@ +From bdf4e332b41ff358b7e8539c3d6ee6277f0be762 Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 6 Jan 2021 09:13:19 +0100 +Subject: [PATCH] libsepol/cil: fix NULL pointer dereference when parsing an + improper integer + +OSS-Fuzz found a NULL pointer dereference when the CIL compiler tries to +compile a policy with an invalid integer: + + $ echo '(ioportcon(2())n)' > tmp.cil + $ secilc tmp.cil + Segmentation fault (core dumped) + +This is because strtol() is called with a NULL pointer, in +cil_fill_integer(). + +Fix this by checking that int_node->data is not NULL. While at it, use +strtoul() instead of strtol() to parse an unsigned integer. + +When using "val > UINT32_MAX" with "unsigned long val;", it is expected +that some compilers emit a warning when the size of "unsigned long" is +32 bits. In theory gcc could be such a compiler (with warning +-Wtype-limits, which is included in -Wextra). Nevertheless this is +currently broken, according to +https://gcc.gnu.org/pipermail/gcc-help/2021-January/139755.html and +https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89126 (this bug was +opened in January 2019). + +In order to prevent this warning from appearing, introduce some +preprocessor macros around the bound check. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28456 +Signed-off-by: Nicolas Iooss +Acked-by: James Carter +--- + libsepol/cil/src/cil_build_ast.c | 16 ++++++++++++---- + 1 file changed, 12 insertions(+), 4 deletions(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index be10d61b1..02481558a 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -5570,19 +5570,27 @@ int cil_fill_integer(struct cil_tree_node *int_node, uint32_t *integer, int base + { + int rc = SEPOL_ERR; + char *endptr = NULL; +- int val; ++ unsigned long val; + +- if (int_node == NULL || integer == NULL) { ++ if (int_node == NULL || int_node->data == NULL || integer == NULL) { + goto exit; + } + + errno = 0; +- val = strtol(int_node->data, &endptr, base); ++ val = strtoul(int_node->data, &endptr, base); + if (errno != 0 || endptr == int_node->data || *endptr != '\0') { + rc = SEPOL_ERR; + goto exit; + } + ++ /* Ensure that the value fits a 32-bit integer without triggering -Wtype-limits */ ++#if ULONG_MAX > UINT32_MAX ++ if (val > UINT32_MAX) { ++ rc = SEPOL_ERR; ++ goto exit; ++ } ++#endif ++ + *integer = val; + + return SEPOL_OK; +@@ -5598,7 +5606,7 @@ int cil_fill_integer64(struct cil_tree_node *int_node, uint64_t *integer, int ba + char *endptr = NULL; + uint64_t val; + +- if (int_node == NULL || integer == NULL) { ++ if (int_node == NULL || int_node->data == NULL || integer == NULL) { + goto exit; + } + diff --git a/backport-libsepol-cil-fix-NULL-pointer-dereference-when-using.patch b/backport-libsepol-cil-fix-NULL-pointer-dereference-when-using.patch new file mode 100644 index 0000000..140c6b3 --- /dev/null +++ b/backport-libsepol-cil-fix-NULL-pointer-dereference-when-using.patch @@ -0,0 +1,41 @@ +From 38a09b74024bbe1c78639821f3cff3a5ceb73d0d Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 30 Dec 2020 21:11:39 +0100 +Subject: [PATCH] libsepol/cil: fix NULL pointer dereference when using an + unused alias + +OSS-Fuzz found a NULL pointer dereference when the CIL compiler tries to +compile a policy where a categoryalias references an unused +categoryalias: + + $ echo '(categoryalias c0)(categoryalias c1)(categoryaliasactual c0 c1)' > tmp.cil + $ secil tmp.cil + Segmentation fault (core dumped) + +In such a case, a1 can become NULL in cil_resolve_alias_to_actual(). +Add a check to report an error when this occurs. Now the error message +is: + + Alias c0 references an unused alias c1 at tmp.cil:1 + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28471 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_resolve_ast.c | 4 ++++ + 1 file changed, 4 insertions(+) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index f6deb1002..affa7657b 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -555,6 +555,10 @@ int cil_resolve_alias_to_actual(struct cil_tree_node *current, enum cil_flavor f + a1_node = a1->datum.nodes->head->data; + + while (flavor != a1_node->flavor) { ++ if (a1->actual == NULL) { ++ cil_tree_log(current, CIL_ERR, "Alias %s references an unused alias %s", alias->datum.name, a1->datum.name); ++ return SEPOL_ERR; ++ } + a1 = a1->actual; + a1_node = a1->datum.nodes->head->data; + steps += 1; diff --git a/backport-libsepol-cil-fix-out-of-bound-read-in-cil_print_recu.patch b/backport-libsepol-cil-fix-out-of-bound-read-in-cil_print_recu.patch new file mode 100644 index 0000000..84fe457 --- /dev/null +++ b/backport-libsepol-cil-fix-out-of-bound-read-in-cil_print_recu.patch @@ -0,0 +1,57 @@ +From 228c06d97a8a33b60b89ded17acbd0e92cca9cfe Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 30 Dec 2020 11:07:45 +0100 +Subject: [PATCH] libsepol/cil: fix out-of-bound read in + cil_print_recursive_blockinherit + +OSS-Fuzz found a heap buffer overflow (out-of-bound reads) when the CIL +compiler tries to report a recursive blockinherit with an optional +block: + + $ echo '(block b (optional o (blockinherit b)))' > tmp.cil + $ secilc tmp.cil + Segmentation fault (core dumped) + +This is because cil_print_recursive_blockinherit() assumes that all +nodes are either CIL_BLOCK or CIL_BLOCKINHERIT. Add support for other +block kinds, using cil_node_to_string() to show them. + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=28462 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_resolve_ast.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c +index affa7657b..68b590bcc 100644 +--- a/libsepol/cil/src/cil_resolve_ast.c ++++ b/libsepol/cil/src/cil_resolve_ast.c +@@ -2347,11 +2347,13 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ + for (curr = bi_node; curr != terminating_node; curr = curr->parent) { + if (curr->flavor == CIL_BLOCK) { + cil_list_prepend(trace, CIL_NODE, curr); +- } else { ++ } else if (curr->flavor == CIL_BLOCKINHERIT) { + if (curr != bi_node) { + cil_list_prepend(trace, CIL_NODE, NODE(((struct cil_blockinherit *)curr->data)->block)); + } + cil_list_prepend(trace, CIL_NODE, curr); ++ } else { ++ cil_list_prepend(trace, CIL_NODE, curr); + } + } + cil_list_prepend(trace, CIL_NODE, terminating_node); +@@ -2360,8 +2362,12 @@ void cil_print_recursive_blockinherit(struct cil_tree_node *bi_node, struct cil_ + curr = item->data; + if (curr->flavor == CIL_BLOCK) { + cil_tree_log(curr, CIL_ERR, "block %s", DATUM(curr->data)->name); +- } else { ++ } else if (curr->flavor == CIL_BLOCKINHERIT) { + cil_tree_log(curr, CIL_ERR, "blockinherit %s", ((struct cil_blockinherit *)curr->data)->block_str); ++ } else if (curr->flavor == CIL_OPTIONAL) { ++ cil_tree_log(curr, CIL_ERR, "optional %s", DATUM(curr->data)->name); ++ } else { ++ cil_tree_log(curr, CIL_ERR, "%s", cil_node_to_string(curr)); + } + } + diff --git a/backport-libsepol-cil-propagate-failure-of-cil_fill_list.patch b/backport-libsepol-cil-propagate-failure-of-cil_fill_list.patch new file mode 100644 index 0000000..7caf5df --- /dev/null +++ b/backport-libsepol-cil-propagate-failure-of-cil_fill_list.patch @@ -0,0 +1,49 @@ +From e2d018423d5910e88947bba3b96d2f301d890c62 Mon Sep 17 00:00:00 2001 +From: Nicolas Iooss +Date: Wed, 30 Dec 2020 21:11:41 +0100 +Subject: [PATCH] libsepol/cil: propagate failure of cil_fill_list() + +OSS-Fuzz found a Null-dereference READ in the CIL compiler when trying +to compile the following policy: + + (optional o (validatetrans x (eq t3 (a ())))) + +With some logs, secilc reports: + + Invalid syntax + Destroying Parse Tree + Resolving AST + Failed to resolve validatetrans statement at fuzz:1 + Disabling optional 'o' at tmp.cil:1 + +So there is an "Invalid syntax" error, but the compilation continues. +Fix this issue by stopping the compilation when cil_fill_list() reports +an error: + + Invalid syntax + Bad expression tree for constraint + Bad validatetrans declaration at tmp.cil:1 + +Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=29061 +Signed-off-by: Nicolas Iooss +--- + libsepol/cil/src/cil_build_ast.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c +index d5b9977c0..be10d61b1 100644 +--- a/libsepol/cil/src/cil_build_ast.c ++++ b/libsepol/cil/src/cil_build_ast.c +@@ -2713,7 +2713,11 @@ static int __cil_fill_constraint_leaf_expr(struct cil_tree_node *current, enum c + cil_list_append(*leaf_expr, CIL_STRING, current->next->next->data); + } else if (r_flavor == CIL_LIST) { + struct cil_list *sub_list; +- cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); ++ rc = cil_fill_list(current->next->next->cl_head, leaf_expr_flavor, &sub_list); ++ if (rc != SEPOL_OK) { ++ cil_list_destroy(leaf_expr, CIL_TRUE); ++ goto exit; ++ } + cil_list_append(*leaf_expr, CIL_LIST, sub_list); + } else { + cil_list_append(*leaf_expr, CIL_CONS_OPERAND, (void *)r_flavor); diff --git a/libsepol.spec b/libsepol.spec index 14579c7..e4e0727 100644 --- a/libsepol.spec +++ b/libsepol.spec @@ -1,12 +1,20 @@ Name: libsepol Version: 3.1 -Release: 2 +Release: 3 Summary: SELinux binary policy manipulation library License: LGPLv2+ URL: https://github.com/SELinuxProject/selinux/wiki/Releases Source0: https://github.com/SELinuxProject/selinux/releases/download/20200710/libsepol-3.1.tar.gz Patch1: backport-libsepol-cil-fix-NULL-pointer-dereference-in-cil_fil.patch +Patch2: backport-libsepol-cil-always-destroy-the-lexer-state.patch +Patch3: backport-libsepol-cil-destroy-perm_datums-when-_cil_resolve_.patch +Patch4: backport-libsepol-cil-do-not-add-a-stack-variable-to-a-list.patch +Patch5: backport-libsepol-cil-Fix-heap-use-after-free-in_class-rese.patch +Patch6: backport-libsepol-cil-fix-NULL-pointer-dereference-when-parsi.patch +Patch7: backport-libsepol-cil-fix-NULL-pointer-dereference-when-using.patch +Patch8: backport-libsepol-cil-fix-out-of-bound-read-in-cil_print_recu.patch +Patch9: backport-libsepol-cil-propagate-failure-of-cil_fill_list.patch BuildRequires: gcc flex @@ -69,6 +77,11 @@ exit 0 %{_mandir}/man3/* %changelog +* Mon Mar 15 2021 yangzhuangzhuang - 3.1-3 +- fix heap-use-after-free in cil_yy_switch_to_buffer + fix heap-use-after-free in __class_reset_perm_values() + fix heap-buffer-overflow in cil_print_recursive_blockinherit + * Thu Mar 4 2021 Lirui - 3.1-2 - fix NULL pointer dereference in cil_fill_ipaddr -- Gitee