[PATCH] libsepol: Improve validation of scopes

[James Carter <jwcart2@gmail.com>]

The validation of scopes can be improved in several ways.

For non-kernel policies and excluding the commons scope table, every
key found in a symbol table should also be in the correspondeing
scope table and vice versa. To validate this, verify that symbol and
scope tables with the same flavor have the same number of elements
and then check that each key found in a scope table can also be found
in the matching symbol table. The function scope_read() in policydb.c
also checks that scope keys can be found in the symbol tables, so
remove that check to consolidate validation.

The commons scope table is always be empty for all policies, so fail
validation if it is not.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/src/policydb.c          |  5 ----
 libsepol/src/policydb_validate.c | 39 ++++++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
index 3dd3ac4f..81c8da82 100644
--- a/libsepol/src/policydb.c
+++ b/libsepol/src/policydb.c
@@ -3903,11 +3903,6 @@ static int scope_read(policydb_t *p, int symnum, struct policy_file *fp)
 	if (rc < 0)
 		goto cleanup;
 
-	/* ensure that there already exists a symbol with this key */
-	if (hashtab_search(p->symtab[symnum].table, key) == NULL) {
-		goto cleanup;
-	}
-
 	if ((scope = calloc(1, sizeof(*scope))) == NULL) {
 		goto cleanup;
 	}
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index 34182e8e..5b2e94ea 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -38,6 +38,11 @@ typedef struct perm_arg {
 	const uint32_t inherited_nprim;
 } perm_arg_t;
 
+typedef struct scope_arg {
+	const symtab_t *symtab;
+	uint32_t num_decls;
+} scope_arg_t;
+
 static int create_gap_ebitmap(char **val_to_name, uint32_t nprim,
 			      ebitmap_t *gaps)
 {
@@ -210,11 +215,10 @@ bad:
 	return -1;
 }
 
-static int validate_scope(__attribute__((unused)) hashtab_key_t k,
-			  hashtab_datum_t d, void *args)
+static int validate_scope(hashtab_key_t k, hashtab_datum_t d, void *args)
 {
 	const scope_datum_t *scope_datum = (scope_datum_t *)d;
-	const uint32_t *nprim = (uint32_t *)args;
+	scope_arg_t *sargs = (scope_arg_t *)args;
 	uint32_t i;
 
 	switch (scope_datum->scope) {
@@ -226,10 +230,13 @@ static int validate_scope(__attribute__((unused)) hashtab_key_t k,
 	}
 
 	for (i = 0; i < scope_datum->decl_ids_len; i++) {
-		if (!value_isvalid(scope_datum->decl_ids[i], *nprim))
+		if (!value_isvalid(scope_datum->decl_ids[i], sargs->num_decls))
 			goto bad;
 	}
 
+	if (!hashtab_search(sargs->symtab->table, k))
+		goto bad;
+
 	return 0;
 
 bad:
@@ -237,20 +244,34 @@ bad:
 }
 
 static int validate_scopes(sepol_handle_t *handle, const symtab_t scopes[],
-			   const avrule_block_t *block)
+			   const symtab_t symtabs[], const policydb_t *p)
 {
+	scope_arg_t sargs;
+	const avrule_block_t *block;
 	const avrule_decl_t *decl;
 	unsigned int i;
 	uint32_t num_decls = 0;
 
-	for (; block != NULL; block = block->next) {
+	for (block = p->global; block != NULL; block = block->next) {
 		for (decl = block->branch_list; decl; decl = decl->next) {
 			num_decls++;
 		}
 	}
 
-	for (i = 0; i < SYM_NUM; i++) {
-		if (hashtab_map(scopes[i].table, validate_scope, &num_decls))
+	if (scopes[SYM_COMMONS].table->nel != 0)
+		goto bad;
+
+	if (p->policy_type != POLICY_KERN) {
+		for (i = 1; i < SYM_NUM; i++) {
+			if (scopes[i].table->nel != symtabs[i].table->nel)
+				goto bad;
+		}
+	}
+
+	sargs.num_decls = num_decls;
+	for (i = 1; i < SYM_NUM; i++) {
+		sargs.symtab = &symtabs[i];
+		if (hashtab_map(scopes[i].table, validate_scope, &sargs))
 			goto bad;
 	}
 
@@ -2152,7 +2173,7 @@ int policydb_validate(sepol_handle_t *handle, const policydb_t *p)
 	if (validate_genfs(handle, p, flavors))
 		goto bad;
 
-	if (validate_scopes(handle, p->scope, p->global))
+	if (validate_scopes(handle, p->scope, p->symtab, p))
 		goto bad;
 
 	if (validate_datum_array_gaps(handle, p, flavors))
-- 
2.55.0