[PATCH] libsepol/cil: Check for duplicate blocks, optionals, and macros

* [PATCH] libsepol/cil: Check for duplicate blocks, optionals, and macros
@ 2021-03-16 16:51 James Carter
  2021-03-16 20:48 ` Nicolas Iooss
  0 siblings, 1 reply; 3+ messages in thread
From: James Carter @ 2021-03-16 16:51 UTC (permalink / raw)
  To: selinux; +Cc: James Carter, Nicolas Iooss, Evgeny Vereshchagin

In CIL, blocks, optionals, and macros share the same symbol table so
that the targets of "in" statements can be located. Because of this,
they cannot have the same name in the same namespace, but, because
they do not show up in the final policy, they can have the same name
as long as they are in different namespaces. Unfortunately, when
copying from one namespace to another, no check was being done to see
if there was a conflict.

When copying blocks, optionals, and macros, if a datum is found in
the destination namespace, then there is a conflict with a previously
declared block, optional, or macro, so exit with an error.

Reported-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Reported-by: Evgeny Vereshchagin <evvers@ya.ru>
Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/cil/src/cil_copy_ast.c | 89 +++++++++------------------------
 1 file changed, 25 insertions(+), 64 deletions(-)

diff --git a/libsepol/cil/src/cil_copy_ast.c b/libsepol/cil/src/cil_copy_ast.c
index c9aada9d..ed967861 100644
--- a/libsepol/cil/src/cil_copy_ast.c
+++ b/libsepol/cil/src/cil_copy_ast.c
@@ -100,16 +100,17 @@ int cil_copy_block(__attribute__((unused)) struct cil_db *db, void *data, void *
 	struct cil_block *orig = data;
 	char *key = orig->datum.name;
 	struct cil_symtab_datum *datum = NULL;
+	struct cil_block *new;
 
 	cil_symtab_get_datum(symtab, key, &datum);
-	if (datum == NULL) {
-		struct cil_block *new;
-		cil_block_init(&new);
-		*copy = new;
-	} else {
-		*copy = datum;;
+	if (datum != NULL) {
+		cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
+		return SEPOL_ERR;
 	}
 
+	cil_block_init(&new);
+	*copy = new;
+
 	return SEPOL_OK;
 }
 
@@ -1509,64 +1510,22 @@ int cil_copy_macro(__attribute__((unused)) struct cil_db *db, void *data, void *
 	struct cil_macro *orig = data;
 	char *key = orig->datum.name;
 	struct cil_symtab_datum *datum = NULL;
+	struct cil_macro *new;
 
 	cil_symtab_get_datum(symtab, key, &datum);
-	if (datum == NULL) {
-		struct cil_macro *new;
-		cil_macro_init(&new);
-		if (orig->params != NULL) {
-			cil_copy_list(orig->params, &new->params);
-		}
-
-		*copy = new;
-
-	} else {
-		struct cil_list_item *curr_orig = NULL;
-		struct cil_list_item *curr_new = NULL;
-		struct cil_param *param_orig = NULL;
-		struct cil_param *param_new = NULL;
-
-		if (((struct cil_macro*)datum)->params != NULL) {
-			curr_new = ((struct cil_macro*)datum)->params->head;
-		}
-
-		if (orig->params != NULL) {
-			curr_orig = orig->params->head;
-		}
-
-		if (curr_orig != NULL && curr_new != NULL) {
-			while (curr_orig != NULL) {
-				if (curr_new == NULL) {
-					goto exit;
-				}
-
-				param_orig = (struct cil_param*)curr_orig->data;
-				param_new = (struct cil_param*)curr_new->data;
-				if (param_orig->str != param_new->str) {
-					goto exit;
-				} else if (param_orig->flavor != param_new->flavor) {
-					goto exit;
-				}
-
-				curr_orig = curr_orig->next;
-				curr_new = curr_new->next;
-			}
-
-			if (curr_new != NULL) {
-				goto exit;
-			}
-		} else if (!(curr_orig == NULL && curr_new == NULL)) {
-			goto exit;
-		}
+	if (datum != NULL) {
+		cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
+		return SEPOL_ERR;
+	}
 
-		*copy = datum;
+	cil_macro_init(&new);
+	if (orig->params != NULL) {
+		cil_copy_list(orig->params, &new->params);
 	}
 
-	return SEPOL_OK;
+	*copy = new;
 
-exit:
-	cil_log(CIL_INFO, "cil_copy_macro: macro cannot be redefined\n");
-	return SEPOL_ERR;
+	return SEPOL_OK;
 }
 
 int cil_copy_optional(__attribute__((unused)) struct cil_db *db, void *data, void **copy, symtab_t *symtab)
@@ -1574,16 +1533,17 @@ int cil_copy_optional(__attribute__((unused)) struct cil_db *db, void *data, voi
 	struct cil_optional *orig = data;
 	char *key = orig->datum.name;
 	struct cil_symtab_datum *datum = NULL;
+	struct cil_optional *new;
 
 	cil_symtab_get_datum(symtab, key, &datum);
-	if (datum == NULL) {
-		struct cil_optional *new;
-		cil_optional_init(&new);
-		*copy = new;
-	} else {
-		*copy = datum;
+	if (datum != NULL) {
+		cil_tree_log(NODE(datum), CIL_ERR, "Re-declaration of %s %s", cil_node_to_string(NODE(datum)), key);
+		return SEPOL_ERR;
 	}
 
+	cil_optional_init(&new);
+	*copy = new;
+
 	return SEPOL_OK;
 }
 
@@ -2122,6 +2082,7 @@ int __cil_copy_node_helper(struct cil_tree_node *orig, __attribute__((unused)) u
 			args->dest = new;
 		}
 	} else {
+		cil_tree_log(orig, CIL_ERR, "Problem copying %s node", cil_node_to_string(orig));
 		goto exit;
 	}
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread