dcbb3a6a58
fix heap-use-after-free in __class_reset_perm_values() fix heap-buffer-overflow in cil_print_recursive_blockinherit
91 lines
3.5 KiB
Diff
91 lines
3.5 KiB
Diff
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;
|