SELinux Security Module development
 help / color / mirror / Atom feed
From: James Carter <jwcart2@gmail.com>
To: selinux@vger.kernel.org
Cc: nicolas.iooss@m4x.org, James Carter <jwcart2@gmail.com>
Subject: [PATCH 4/5] libsepol/cil: Check for self-referential loops in sets
Date: Wed, 28 Apr 2021 16:17:52 -0400	[thread overview]
Message-ID: <20210428201753.282831-5-jwcart2@gmail.com> (raw)
In-Reply-To: <20210428201753.282831-1-jwcart2@gmail.com>

The secilc-fuzzer found a self-referential loop using category sets.
Any set declaration in CIL that allows sets in it is susceptible to
the creation of a self-referential loop. There is a check, but only
for the name of the set being declared being used in the set
declaration.

Check for self-refential loops in user, role, and type attributes
and in category sets. Since all of the sets need to be declared,
this check has to be done when verifying the CIL db before doing
the post phase.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_resolve_ast.c | 31 +---------
 libsepol/cil/src/cil_verify.c      | 97 +++++++++++++++++++++---------
 libsepol/cil/src/cil_verify.h      |  1 -
 3 files changed, 71 insertions(+), 58 deletions(-)

diff --git a/libsepol/cil/src/cil_resolve_ast.c b/libsepol/cil/src/cil_resolve_ast.c
index b081d45d..34c28fc6 100644
--- a/libsepol/cil/src/cil_resolve_ast.c
+++ b/libsepol/cil/src/cil_resolve_ast.c
@@ -438,11 +438,6 @@ int cil_resolve_typeattributeset(struct cil_tree_node *current, void *extra_args
 		goto exit;
 	}
 
-	rc = cil_verify_no_self_reference(attr_datum, attrtypes->datum_expr);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
 	if (attr->expr_list == NULL) {
 		cil_list_init(&attr->expr_list, CIL_TYPEATTRIBUTE);
 	}
@@ -1151,11 +1146,6 @@ int cil_resolve_roleattributeset(struct cil_tree_node *current, void *extra_args
 		goto exit;
 	}
 
-	rc = cil_verify_no_self_reference(attr_datum, attrroles->datum_expr);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
 	if (attr->expr_list == NULL) {
 		cil_list_init(&attr->expr_list, CIL_ROLEATTRIBUTE);
 	}
@@ -1666,21 +1656,7 @@ exit:
 
 int cil_resolve_catset(struct cil_tree_node *current, struct cil_catset *catset, void *extra_args)
 {
-	int rc = SEPOL_ERR;
-
-	rc = cil_resolve_cats(current, catset->cats, extra_args);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
-	rc = cil_verify_no_self_reference((struct cil_symtab_datum *)catset, catset->cats->datum_expr);
-	if (rc != SEPOL_OK) {
-		cil_list_destroy(&catset->cats->datum_expr, CIL_FALSE);
-		goto exit;
-	}
-
-exit:
-	return rc;
+	return cil_resolve_cats(current, catset->cats, extra_args);
 }
 
 int cil_resolve_senscat(struct cil_tree_node *current, void *extra_args)
@@ -3544,11 +3520,6 @@ int cil_resolve_userattributeset(struct cil_tree_node *current, void *extra_args
 		goto exit;
 	}
 
-	rc = cil_verify_no_self_reference(attr_datum, attrusers->datum_expr);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
 	if (attr->expr_list == NULL) {
 		cil_list_init(&attr->expr_list, CIL_USERATTRIBUTE);
 	}
diff --git a/libsepol/cil/src/cil_verify.c b/libsepol/cil/src/cil_verify.c
index 5a37dd2f..4c860a21 100644
--- a/libsepol/cil/src/cil_verify.c
+++ b/libsepol/cil/src/cil_verify.c
@@ -430,28 +430,71 @@ int cil_verify_decl_does_not_shadow_macro_parameter(struct cil_macro *macro, str
 	return SEPOL_OK;
 }
 
-int cil_verify_no_self_reference(struct cil_symtab_datum *datum, struct cil_list *datum_list)
+int cil_verify_no_self_reference(enum cil_flavor flavor, struct cil_symtab_datum *datum, struct cil_symtab_datum *orig);
+
+static int __verify_no_self_reference_in_expr(struct cil_list *expr, struct cil_symtab_datum *orig)
 {
-	struct cil_list_item *i;
+	struct cil_list_item *item;
+	int rc = SEPOL_OK;
 
-	cil_list_for_each(i, datum_list) {
-		if (i->flavor == CIL_DATUM) {
-			struct cil_symtab_datum *d = i->data;
-			if (d == datum) {
-				cil_log(CIL_ERR,"Self-reference found for %s\n",datum->name);
-				return SEPOL_ERR;
-			}
-		} else if (i->flavor == CIL_LIST) {
-			int rc = cil_verify_no_self_reference(datum, i->data);
-			if (rc != SEPOL_OK) {
-				return SEPOL_ERR;
-			}
+	if (!expr) {
+		return SEPOL_OK;
+	}
+
+	cil_list_for_each(item, expr) {
+		if (item->flavor == CIL_DATUM) {
+			struct cil_symtab_datum* datum = item->data;
+			rc = cil_verify_no_self_reference(FLAVOR(datum), datum, orig);
+		} else if (item->flavor == CIL_LIST) {
+			rc = __verify_no_self_reference_in_expr(item->data, orig);
+		}
+		if (rc != SEPOL_OK) {
+			return SEPOL_ERR;
 		}
 	}
 
 	return SEPOL_OK;
 }
 
+int cil_verify_no_self_reference(enum cil_flavor flavor, struct cil_symtab_datum *datum, struct cil_symtab_datum *orig)
+{
+	int rc = SEPOL_OK;
+
+	if (datum == orig) {
+		cil_tree_log(NODE(orig), CIL_ERR, "Self-reference found for %s", orig->name);
+		return SEPOL_ERR;
+	} else if (orig == NULL) {
+		orig = datum;
+	}
+
+	switch (flavor) {
+	case CIL_USERATTRIBUTE: {
+		struct cil_userattribute *attr = (struct cil_userattribute *)datum;
+		rc = __verify_no_self_reference_in_expr(attr->expr_list, orig);
+		break;
+	}
+	case CIL_ROLEATTRIBUTE: {
+		struct cil_roleattribute *attr = (struct cil_roleattribute *)datum;
+		rc = __verify_no_self_reference_in_expr(attr->expr_list, orig);
+		break;
+	}
+	case CIL_TYPEATTRIBUTE: {
+		struct cil_typeattribute *attr = (struct cil_typeattribute *)datum;
+		rc = __verify_no_self_reference_in_expr(attr->expr_list, orig);
+		break;
+	}
+	case CIL_CATSET: {
+		struct cil_catset *set = (struct cil_catset *)datum;
+		rc = __verify_no_self_reference_in_expr(set->cats->datum_expr, orig);
+		break;
+	}
+	default:
+		break;
+	}
+
+	return rc;
+}
+
 int __cil_verify_ranges(struct cil_list *list)
 {
 	int rc = SEPOL_ERR;
@@ -1757,27 +1800,22 @@ static int __cil_verify_map_class(struct cil_tree_node *node)
 
 int __cil_pre_verify_helper(struct cil_tree_node *node, uint32_t *finished, __attribute__((unused)) void *extra_args)
 {
-	int rc = SEPOL_ERR;
+	int rc = SEPOL_OK;
 
-	if (node->flavor == CIL_MACRO) {
+	switch (node->flavor) {
+	case CIL_MACRO: {
 		*finished = CIL_TREE_SKIP_HEAD;
-		rc = SEPOL_OK;
-		goto exit;
-	} else if (node->flavor == CIL_BLOCK) {
+		break;
+	}
+	case CIL_BLOCK: {
 		struct cil_block *blk = node->data;
 		if (blk->is_abstract == CIL_TRUE) {
 			*finished = CIL_TREE_SKIP_HEAD;
 		}
-		rc = SEPOL_OK;
-		goto exit;
+		break;
 	}
-
-	switch (node->flavor) {
 	case CIL_USER:
 		rc = __cil_verify_user_pre_eval(node);
-		if (rc != SEPOL_OK) {
-			goto exit;
-		}
 		break;
 	case CIL_MAP_CLASS:
 		rc = __cil_verify_map_class(node);
@@ -1785,11 +1823,16 @@ int __cil_pre_verify_helper(struct cil_tree_node *node, uint32_t *finished, __at
 	case CIL_CLASSPERMISSION:
 		rc = __cil_verify_classpermission(node);
 		break;
+	case CIL_USERATTRIBUTE:
+	case CIL_ROLEATTRIBUTE:
+	case CIL_TYPEATTRIBUTE:
+	case CIL_CATSET:
+		rc = cil_verify_no_self_reference(node->flavor, node->data, NULL);
+		break;
 	default:
 		rc = SEPOL_OK;
 		break;
 	}
 
-exit:
 	return rc;
 }
diff --git a/libsepol/cil/src/cil_verify.h b/libsepol/cil/src/cil_verify.h
index c497018f..4ea14f5b 100644
--- a/libsepol/cil/src/cil_verify.h
+++ b/libsepol/cil/src/cil_verify.h
@@ -63,7 +63,6 @@ int cil_verify_constraint_leaf_expr_syntax(enum cil_flavor l_flavor, enum cil_fl
 int cil_verify_constraint_expr_syntax(struct cil_tree_node *current, enum cil_flavor op);
 int cil_verify_conditional_blocks(struct cil_tree_node *current);
 int cil_verify_decl_does_not_shadow_macro_parameter(struct cil_macro *macro, struct cil_tree_node *node, const char *name);
-int cil_verify_no_self_reference(struct cil_symtab_datum *datum, struct cil_list *datum_list);
 int __cil_verify_ranges(struct cil_list *list);
 int __cil_verify_ordered_node_helper(struct cil_tree_node *node, uint32_t *finished, void *extra_args);
 int __cil_verify_ordered(struct cil_tree_node *current, enum cil_flavor flavor);
-- 
2.26.3


  parent reply	other threads:[~2021-04-28 20:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-28 20:17 [PATCH 0/5] Fix bugs identified by the secilc-fuzzer James Carter
2021-04-28 20:17 ` [PATCH 1/5] libsepol/cil: Fix instances where an error returns SEPOL_OK James Carter
2021-04-28 20:17 ` [PATCH 2/5] libsepol/cil: Detect degenerate inheritance and exit with an error James Carter
2021-04-28 20:17 ` [PATCH 3/5] libsepol/cil: Check datum in ordered list for expected flavor James Carter
2021-04-28 20:17 ` James Carter [this message]
2021-05-01 14:33   ` [PATCH 4/5] libsepol/cil: Check for self-referential loops in sets Nicolas Iooss
2021-05-03 13:48     ` James Carter
2021-04-28 20:17 ` [PATCH 5/5] libsepol/cil: Return an error if a call argument fails to resolve James Carter
2021-05-04 20:07 ` [PATCH 0/5] Fix bugs identified by the secilc-fuzzer James Carter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210428201753.282831-5-jwcart2@gmail.com \
    --to=jwcart2@gmail.com \
    --cc=nicolas.iooss@m4x.org \
    --cc=selinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox