From e5ae2662ecdf9534b77acfa1c222e656bc7fdd8a 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 43e6b88e0f..52e5f64011 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;