[PATCH] libsepol/cil: Fix double free in cil_deny.c

The function cil_create_attribute_d1_and_not_d2() (let's call this
f1) mallocs an ebitmap named types. This ebitmap is passed to the
function cil_create_and_insert_attribute_and_set() (let's call this
f2) which will create a new typeattribute which has its types field
point to the types ebitmap passed in by f1. If there is an error in
getting a symtab or inserting the new attribute into the symtab,
then cil_destroy_typeattribute() is called during cleanup. This will
free the ebitmap pointed to by the types field. After f2 exits with
an error, f1 will perform cleanup and try to free the types ebitmap
again.

Move the calls to cil_get_symtab() and cil_symtab_insert() to
earlier in f2 to make it easier to clean up in the case of an error.
When cleaning up from an error do the cleanup manually instead of
calling cil_destroy_typeattribute() and do not free the passed in
types ebitmap. Function f1 needs to call ebitmap_destroy() (which
is safe to call more than once on a ebitmap) and free the ebitmap.

Also add ebitmap_destroy() calls to
cil_create_attribute_all_and_not_d() and
cil_create_attribute_d1_and_d2()

Reported-by: oss-fuzz (issue 500754862)
Signed-off-by: James Carter <[email protected]>
---
 libsepol/cil/src/cil_deny.c | 44 ++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/libsepol/cil/src/cil_deny.c b/libsepol/cil/src/cil_deny.c
index c81b8beb..dc0f4ae9 100644
--- a/libsepol/cil/src/cil_deny.c
+++ b/libsepol/cil/src/cil_deny.c
@@ -870,7 +870,6 @@ static int cil_create_and_insert_attribute_and_set(struct cil_db *db,
 	struct cil_tree_node *attr_node = NULL;
 	char *name;
 	struct cil_typeattribute *attr = NULL;
-	struct cil_tree_node *attrset_node = NULL;
 	struct cil_typeattributeset *attrset = NULL;
 	symtab_t *symtab = NULL;
 	int rc = SEPOL_ERR;
@@ -880,12 +879,25 @@ static int cil_create_and_insert_attribute_and_set(struct cil_db *db,
 		goto exit;
 	}
 
+	rc = cil_get_symtab(prev->parent, &symtab, CIL_SYM_TYPES);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
+	cil_typeattribute_init(&attr);
+	attr_node = cil_create_and_insert_node(prev, CIL_TYPEATTRIBUTE, attr);
+
+	rc = cil_symtab_insert(symtab, name, &attr->datum, attr_node);
+	if (rc != SEPOL_OK) {
+		goto exit;
+	}
+
 	cil_typeattributeset_init(&attrset);
 	attrset->attr_str = name;
 	attrset->str_expr = str_expr;
 	attrset->datum_expr = datum_expr;
+	cil_create_and_insert_node(attr_node, CIL_TYPEATTRIBUTESET, attrset);
 
-	cil_typeattribute_init(&attr);
 	cil_list_init(&attr->expr_list, CIL_TYPE);
 	cil_list_append(attr->expr_list, CIL_LIST, datum_expr);
 	attr->types = types;
@@ -894,20 +906,6 @@ static int cil_create_and_insert_attribute_and_set(struct cil_db *db,
 			     CIL_FALSE :
 			     CIL_TRUE;
 
-	attr_node = cil_create_and_insert_node(prev, CIL_TYPEATTRIBUTE, attr);
-	attrset_node = cil_create_and_insert_node(
-		attr_node, CIL_TYPEATTRIBUTESET, attrset);
-
-	rc = cil_get_symtab(prev->parent, &symtab, CIL_SYM_TYPES);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
-	rc = cil_symtab_insert(symtab, name, &attr->datum, attr_node);
-	if (rc != SEPOL_OK) {
-		goto exit;
-	}
-
 	db->num_types_and_attrs++;
 
 	*d = &attr->datum;
@@ -915,15 +913,14 @@ static int cil_create_and_insert_attribute_and_set(struct cil_db *db,
 	return SEPOL_OK;
 
 exit:
+	if (attr) {
+		cil_symtab_datum_destroy(&attr->datum);
+		free(attr);
+	}
 	if (attr_node) {
-		cil_destroy_typeattribute(
-			attr_node->data); // This will not destroy datum_expr
+		prev->next = attr_node->next;
 		free(attr_node);
 	}
-	if (attrset_node) {
-		prev->next = attrset_node->next;
-		free(attrset_node);
-	}
 	return rc;
 }
 
@@ -1038,6 +1035,7 @@ static int cil_create_attribute_all_and_not_d(struct cil_db *db,
 exit:
 	cil_list_destroy(&str_expr, CIL_FALSE);
 	cil_list_destroy(&datum_expr, CIL_FALSE);
+	ebitmap_destroy(types);
 	free(types);
 	return rc;
 }
@@ -1103,6 +1101,7 @@ static int cil_create_attribute_d1_and_not_d2(struct cil_db *db,
 exit:
 	cil_list_destroy(&str_expr, CIL_FALSE);
 	cil_list_destroy(&datum_expr, CIL_FALSE);
+	ebitmap_destroy(types);
 	free(types);
 	return rc;
 }
@@ -1168,6 +1167,7 @@ static int cil_create_attribute_d1_and_d2(struct cil_db *db,
 exit:
 	cil_list_destroy(&str_expr, CIL_FALSE);
 	cil_list_destroy(&datum_expr, CIL_FALSE);
+	ebitmap_destroy(types);
 	free(types);
 	return rc;
 }
-- 
2.55.0