From d668f8e3a0a0361c03881ae3f00509196eaee064 Mon Sep 17 00:00:00 2001

From: James Carter <jwcart2@gmail.com>

Date: Mon, 8 Feb 2021 11:23:42 -0500

Subject: [PATCH] libsepol/cil: Destroy disabled optional blocks after pass is

 complete

Nicolas Iooss reports:

  I am continuing to investigate OSS-Fuzz crashes and the following one

  is quite complex. Here is a CIL policy which triggers a

  heap-use-after-free error in the CIL compiler:

  (class CLASS (PERM2))

  (classorder (CLASS))

  (classpermission CLSPRM)

  (optional o

      (mlsvalidatetrans x (domby l1 h1))

      (common CLSCOMMON (PERM1))

      (classcommon CLASS CLSCOMMON)

  )

  (classpermissionset CLSPRM (CLASS (PERM1)))

  The issue is that the mlsvalidatetrans fails to resolve in pass

  CIL_PASS_MISC3, which comes after the resolution of classcommon (in

  pass CIL_PASS_MISC2). So:

  * In pass CIL_PASS_MISC2, the optional block still exists, the

  classcommon is resolved and class CLASS is linked with common

  CLSCOMMON.

  * In pass CIL_PASS_MISC3, the optional block is destroyed, including

  the common CLSCOMMON.

  * When classpermissionset is resolved, function cil_resolve_classperms

  uses "common_symtab = &class->common->perms;", which has been freed.

  The use-after-free issue occurs in __cil_resolve_perms (in

  libsepol/cil/src/cil_resolve_ast.c):

    // common_symtab was freed

    rc = cil_symtab_get_datum(common_symtab, curr->data, &perm_datum);

The fundamental problem here is that when the optional block is

disabled it is immediately destroyed in the middle of the pass, so

the class has not been reset and still refers to the now destroyed

common when the classpermissionset is resolved later in the same pass.

Added a list, disabled_optionals, to struct cil_args_resolve which is

passed when resolving the tree. When optionals are disabled, they are

now added to this list and then are destroyed after the tree has been

reset between passes.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>

Signed-off-by: James Carter <jwcart2@gmail.com>

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

---

 libsepol/cil/src/cil_resolve_ast.c | 11 ++++++++++-

 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c

index ed876260..979fa17d 100644

--- a/libsepol/cil/src/cil_resolve_ast.c

+++ b/libsepol/cil/src/cil_resolve_ast.c

@@ -51,6 +51,7 @@ struct cil_args_resolve {

 	struct cil_db *db;

 	enum cil_pass pass;

 	uint32_t *changed;

+	struct cil_list *disabled_optionals;

 	struct cil_tree_node *optstack;

 	struct cil_tree_node *boolif;

 	struct cil_tree_node *macro;

@@ -3854,7 +3855,7 @@ int __cil_resolve_ast_last_child_helper(struct cil_tree_node *current, void *ext

 		if (((struct cil_optional *)parent->data)->enabled == CIL_FALSE) {

 			*(args->changed) = CIL_TRUE;

-			cil_tree_children_destroy(parent);

+			cil_list_append(args->disabled_optionals, CIL_NODE, parent);

 		}

 		/* pop off the stack */

@@ -3917,6 +3918,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)

 	extra_args.in_list = NULL;

 	extra_args.blockstack = NULL;

+	cil_list_init(&extra_args.disabled_optionals, CIL_NODE);

 	cil_list_init(&extra_args.sidorder_lists, CIL_LIST_ITEM);

 	cil_list_init(&extra_args.classorder_lists, CIL_LIST_ITEM);

 	cil_list_init(&extra_args.unordered_classorder_lists, CIL_LIST_ITEM);

@@ -3984,6 +3986,7 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)

 		}

 		if (changed && (pass > CIL_PASS_CALL1)) {

+			struct cil_list_item *item;

 			/* Need to re-resolve because an optional was disabled that contained

 			 * one or more declarations. We only need to reset to the call1 pass 

 			 * because things done in the preceeding passes aren't allowed in 

@@ -4012,6 +4015,11 @@ int cil_resolve_ast(struct cil_db *db, struct cil_tree_node *current)

 				cil_log(CIL_ERR, "Failed to reset declarations\n");

 				goto exit;

 			}

+			cil_list_for_each(item, extra_args.disabled_optionals) {

+				cil_tree_children_destroy(item->data);

+			}

+			cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE);

+			cil_list_init(&extra_args.disabled_optionals, CIL_NODE);

 		}

 		/* reset the arguments */

@@ -4040,6 +4048,7 @@ exit:

 	__cil_ordered_lists_destroy(&extra_args.catorder_lists);

 	__cil_ordered_lists_destroy(&extra_args.sensitivityorder_lists);

 	__cil_ordered_lists_destroy(&extra_args.unordered_classorder_lists);

+	cil_list_destroy(&extra_args.disabled_optionals, CIL_FALSE);

 	cil_list_destroy(&extra_args.in_list, CIL_FALSE);

 	return rc;

-- 

2.30.2