!10 fix heap-use-after-free in cil_yy_switch_to_buffer ,fix heap-buffer-overflow in cil_print_recursive_blockinherit,fix heap-use-after-free in __class_reset_perm_values()
From: @yang_zhuang_zhuang Reviewed-by: @zhujianwei001 Signed-off-by: @zhujianwei001
This commit is contained in:
commit
8eb5e57dfa
@ -0,0 +1,90 @@
|
|||||||
|
From f0d98f83d28a0cdf437b4cafe229e27cb91eb493 Mon Sep 17 00:00:00 2001
|
||||||
|
From: James Carter <jwcart2@gmail.com>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
Signed-off-by: James Carter <jwcart2@gmail.com>
|
||||||
|
---
|
||||||
|
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;
|
||||||
112
backport-libsepol-cil-always-destroy-the-lexer-state.patch
Normal file
112
backport-libsepol-cil-always-destroy-the-lexer-state.patch
Normal file
@ -0,0 +1,112 @@
|
|||||||
|
From 90809674c13c03e324dff560654d0e686c5fc46b Mon Sep 17 00:00:00 2001
|
||||||
|
From: Evgeny Vereshchagin <evvers@ya.ru>
|
||||||
|
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<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /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<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /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<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /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 <evvers@ya.ru>
|
||||||
|
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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;
|
||||||
@ -0,0 +1,36 @@
|
|||||||
|
From b7ea65f547c67bfbae4ae133052583b090747e5a Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
@ -0,0 +1,39 @@
|
|||||||
|
From 6c8fca10452e445edf52daffc69f2e9dfcdac54c Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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);
|
||||||
|
}
|
||||||
@ -0,0 +1,82 @@
|
|||||||
|
From bdf4e332b41ff358b7e8539c3d6ee6277f0be762 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
Acked-by: James Carter <jwcart2@gmail.com>
|
||||||
|
---
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
@ -0,0 +1,41 @@
|
|||||||
|
From 38a09b74024bbe1c78639821f3cff3a5ceb73d0d Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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;
|
||||||
@ -0,0 +1,57 @@
|
|||||||
|
From 228c06d97a8a33b60b89ded17acbd0e92cca9cfe Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@ -0,0 +1,49 @@
|
|||||||
|
From e2d018423d5910e88947bba3b96d2f301d890c62 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Nicolas Iooss <nicolas.iooss@m4x.org>
|
||||||
|
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 <nicolas.iooss@m4x.org>
|
||||||
|
---
|
||||||
|
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);
|
||||||
@ -1,12 +1,20 @@
|
|||||||
Name: libsepol
|
Name: libsepol
|
||||||
Version: 3.1
|
Version: 3.1
|
||||||
Release: 2
|
Release: 3
|
||||||
Summary: SELinux binary policy manipulation library
|
Summary: SELinux binary policy manipulation library
|
||||||
License: LGPLv2+
|
License: LGPLv2+
|
||||||
URL: https://github.com/SELinuxProject/selinux/wiki/Releases
|
URL: https://github.com/SELinuxProject/selinux/wiki/Releases
|
||||||
Source0: https://github.com/SELinuxProject/selinux/releases/download/20200710/libsepol-3.1.tar.gz
|
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
|
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
|
BuildRequires: gcc flex
|
||||||
|
|
||||||
@ -69,6 +77,11 @@ exit 0
|
|||||||
%{_mandir}/man3/*
|
%{_mandir}/man3/*
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
|
* Mon Mar 15 2021 yangzhuangzhuang <yangzhuangzhuang1@huawei.com> - 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 <lirui130@huawei.com> - 3.1-2
|
* Thu Mar 4 2021 Lirui <lirui130@huawei.com> - 3.1-2
|
||||||
- fix NULL pointer dereference in cil_fill_ipaddr
|
- fix NULL pointer dereference in cil_fill_ipaddr
|
||||||
|
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user