Blame SOURCES/0020-libsepol-cil-Check-for-statements-not-allowed-in-opt.patch

212ad1
From 0e420cc6f6debc2050229ea537b592c963b81a7c Mon Sep 17 00:00:00 2001
212ad1
From: James Carter <jwcart2@gmail.com>
212ad1
Date: Tue, 30 Mar 2021 13:39:18 -0400
212ad1
Subject: [PATCH] libsepol/cil: Check for statements not allowed in optional
212ad1
 blocks
212ad1
212ad1
While there are some checks for invalid statements in an optional
212ad1
block when resolving the AST, there are no checks when building the
212ad1
AST.
212ad1
212ad1
OSS-Fuzz found the following policy which caused a null dereference
212ad1
in cil_tree_get_next_path().
212ad1
  (blockinherit b3)
212ad1
  (sid SID)
212ad1
  (sidorder(SID))
212ad1
  (optional o
212ad1
    (ibpkeycon :(1 0)s)
212ad1
    (block b3
212ad1
      (filecon""block())
212ad1
      (filecon""block())))
212ad1
212ad1
The problem is that the blockinherit copies block b3 before
212ad1
the optional block is disabled. When the optional is disabled,
212ad1
block b3 is deleted along with everything else in the optional.
212ad1
Later, when filecon statements with the same path are found an
212ad1
error message is produced and in trying to find out where the block
212ad1
was copied from, the reference to the deleted block is used. The
212ad1
error handling code assumes (rightly) that if something was copied
212ad1
from a block then that block should still exist.
212ad1
212ad1
It is clear that in-statements, blocks, and macros cannot be in an
212ad1
optional, because that allows nodes to be copied from the optional
212ad1
block to somewhere outside even though the optional could be disabled
212ad1
later. When optionals are disabled the AST is reset and the
212ad1
resolution is restarted at the point of resolving macro calls, so
212ad1
anything resolved before macro calls will never be re-resolved.
212ad1
This includes tunableifs, in-statements, blockinherits,
212ad1
blockabstracts, and macro definitions. Tunable declarations also
212ad1
cannot be in an optional block because they are needed to resolve
212ad1
tunableifs. It should be fine to allow blockinherit statements in
212ad1
an optional, because that is copying nodes from outside the optional
212ad1
to the optional and if the optional is later disabled, everything
212ad1
will be deleted anyway.
212ad1
212ad1
Check and quit with an error if a tunable declaration, in-statement,
212ad1
block, blockabstract, or macro definition is found within an
212ad1
optional when either building or resolving the AST.
212ad1
212ad1
Signed-off-by: James Carter <jwcart2@gmail.com>
212ad1
---
212ad1
 libsepol/cil/src/cil_build_ast.c   | 32 ++++++++++++++++++++++++++++++
212ad1
 libsepol/cil/src/cil_resolve_ast.c |  4 +++-
212ad1
 2 files changed, 35 insertions(+), 1 deletion(-)
212ad1
212ad1
diff --git a/libsepol/cil/src/cil_build_ast.c b/libsepol/cil/src/cil_build_ast.c
212ad1
index 3a91be03..4f72884c 100644
212ad1
--- a/libsepol/cil/src/cil_build_ast.c
212ad1
+++ b/libsepol/cil/src/cil_build_ast.c
212ad1
@@ -52,6 +52,7 @@ struct cil_args_build {
212ad1
 	struct cil_tree_node *tunif;
212ad1
 	struct cil_tree_node *in;
212ad1
 	struct cil_tree_node *macro;
212ad1
+	struct cil_tree_node *optional;
212ad1
 	struct cil_tree_node *boolif;
212ad1
 };
212ad1
 
212ad1
@@ -6077,6 +6078,7 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f
212ad1
 	struct cil_tree_node *tunif = args->tunif;
212ad1
 	struct cil_tree_node *in = args->in;
212ad1
 	struct cil_tree_node *macro = args->macro;
212ad1
+	struct cil_tree_node *optional = args->optional;
212ad1
 	struct cil_tree_node *boolif = args->boolif;
212ad1
 	struct cil_tree_node *ast_node = NULL;
212ad1
 	int rc = SEPOL_ERR;
212ad1
@@ -6127,6 +6129,18 @@ int __cil_build_ast_node_helper(struct cil_tree_node *parse_current, uint32_t *f
212ad1
 		}
212ad1
 	}
212ad1
 
212ad1
+	if (optional != NULL) {
212ad1
+		if (parse_current->data == CIL_KEY_TUNABLE ||
212ad1
+			parse_current->data == CIL_KEY_IN ||
212ad1
+			parse_current->data == CIL_KEY_BLOCK ||
212ad1
+			parse_current->data == CIL_KEY_BLOCKABSTRACT ||
212ad1
+			parse_current->data == CIL_KEY_MACRO) {
212ad1
+			rc = SEPOL_ERR;
212ad1
+			cil_tree_log(parse_current, CIL_ERR, "%s is not allowed in optionals", (char *)parse_current->data);
212ad1
+			goto exit;
212ad1
+		}
212ad1
+	}
212ad1
+
212ad1
 	if (boolif != NULL) {
212ad1
 		if (parse_current->data != CIL_KEY_TUNABLEIF &&
212ad1
 			parse_current->data != CIL_KEY_CALL &&
212ad1
@@ -6468,6 +6482,10 @@ int __cil_build_ast_first_child_helper(__attribute__((unused)) struct cil_tree_n
212ad1
 		args->macro = ast;
212ad1
 	}
212ad1
 
212ad1
+	if (ast->flavor == CIL_OPTIONAL) {
212ad1
+		args->optional = ast;
212ad1
+	}
212ad1
+
212ad1
 	if (ast->flavor == CIL_BOOLEANIF) {
212ad1
 		args->boolif = ast;
212ad1
 	}
212ad1
@@ -6498,6 +6516,19 @@ int __cil_build_ast_last_child_helper(struct cil_tree_node *parse_current, void
212ad1
 		args->macro = NULL;
212ad1
 	}
212ad1
 
212ad1
+	if (ast->flavor == CIL_OPTIONAL) {
212ad1
+		struct cil_tree_node *n = ast->parent;
212ad1
+		args->optional = NULL;
212ad1
+		/* Optionals can be nested */
212ad1
+		while (n && n->flavor != CIL_ROOT) {
212ad1
+			if (n->flavor == CIL_OPTIONAL) {
212ad1
+				args->optional = n;
212ad1
+				break;
212ad1
+			}
212ad1
+			n = n->parent;
212ad1
+		}
212ad1
+	}
212ad1
+
212ad1
 	if (ast->flavor == CIL_BOOLEANIF) {
212ad1
 		args->boolif = NULL;
212ad1
 	}
212ad1
@@ -6526,6 +6557,7 @@ int cil_build_ast(struct cil_db *db, struct cil_tree_node *parse_tree, struct ci
212ad1
 	extra_args.tunif = NULL;
212ad1
 	extra_args.in = NULL;
212ad1
 	extra_args.macro = NULL;
212ad1
+	extra_args.optional = NULL;
212ad1
 	extra_args.boolif = NULL;
212ad1
 
212ad1
 	rc = cil_tree_walk(parse_tree, __cil_build_ast_node_helper, __cil_build_ast_first_child_helper, __cil_build_ast_last_child_helper, &extra_args);
212ad1
diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
212ad1
index 06b6ab48..8ffab438 100644
212ad1
--- a/libsepol/cil/src/cil_resolve_ast.c
212ad1
+++ b/libsepol/cil/src/cil_resolve_ast.c
212ad1
@@ -3723,8 +3723,10 @@ int __cil_resolve_ast_node_helper(struct cil_tree_node *node, uint32_t *finished
212ad1
 
212ad1
 	if (optional != NULL) {
212ad1
 		if (node->flavor == CIL_TUNABLE ||
212ad1
+			node->flavor == CIL_IN ||
212ad1
+			node->flavor == CIL_BLOCK ||
212ad1
+			node->flavor == CIL_BLOCKABSTRACT ||
212ad1
 		    node->flavor == CIL_MACRO) {
212ad1
-			/* tuanbles and macros are not allowed in optionals*/
212ad1
 			cil_tree_log(node, CIL_ERR, "%s statement is not allowed in optionals", cil_node_to_string(node));
212ad1
 			rc = SEPOL_ERR;
212ad1
 			goto exit;
212ad1
-- 
212ad1
2.30.2
212ad1