From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
Validate the symbol tables for permissions of security classes and
common classes:
* check their value is valid
* check their values are unique
* check permission values of classes do not reuse values from
inherited permissions
This simplifies validating permissions of access vectors a lot, since it
is now only a binary and against the valid permission mask of the class.
Use UINT32_MAX instead of 0 as the special value for validating
constraints signaling a validate-trans rule, since classes with no
permissions are permitted, but they must not have a normal constraint
attached.
Reported-by: oss-fuzz (issue 67893)
Improves: 8c64e5bb6fe7 ("libsepol: validate access vector permissions")
Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
---
v2:
- move check independent of individual constraints out of the loop
- change nperms parameter type of validate_constraint_nodes() from
unsigned int to uint32_t
Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
---
libsepol/src/policydb_validate.c | 104 ++++++++++++++++++++-----------
1 file changed, 68 insertions(+), 36 deletions(-)
diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
index c4f8c300..e1623172 100644
--- a/libsepol/src/policydb_validate.c
+++ b/libsepol/src/policydb_validate.c
@@ -11,6 +11,7 @@
#define bool_xor(a, b) (!(a) != !(b))
#define bool_xnor(a, b) (!bool_xor(a, b))
+#define PERMISSION_MASK(nprim) ((nprim) == PERM_SYMTAB_SIZE ? (~UINT32_C(0)) : ((UINT32_C(1) << (nprim)) - 1))
typedef struct validate {
uint32_t nprim;
@@ -23,6 +24,12 @@ typedef struct map_arg {
const policydb_t *policy;
} map_arg_t;
+typedef struct perm_arg {
+ uint32_t visited;
+ const uint32_t nprim;
+ const uint32_t inherited_nprim;
+} perm_arg_t;
+
static int create_gap_ebitmap(char **val_to_name, uint32_t nprim, ebitmap_t *gaps)
{
uint32_t i;
@@ -227,17 +234,21 @@ bad:
return -1;
}
-static int validate_constraint_nodes(sepol_handle_t *handle, unsigned int nperms, const constraint_node_t *cons, validate_t flavors[])
+static int validate_constraint_nodes(sepol_handle_t *handle, uint32_t nperms, const constraint_node_t *cons, validate_t flavors[])
{
const constraint_expr_t *cexp;
+ const int is_validatetrans = (nperms == UINT32_MAX);
int depth;
+ if (cons && nperms == 0)
+ goto bad;
+
for (; cons; cons = cons->next) {
- if (nperms == 0 && cons->permissions != 0)
+ if (is_validatetrans && cons->permissions != 0)
goto bad;
- if (nperms > 0 && cons->permissions == 0)
+ if (!is_validatetrans && cons->permissions == 0)
goto bad;
- if (nperms > 0 && nperms != PERM_SYMTAB_SIZE && cons->permissions >= (UINT32_C(1) << nperms))
+ if (!is_validatetrans && nperms != PERM_SYMTAB_SIZE && cons->permissions >= (UINT32_C(1) << nperms))
goto bad;
if (!cons->expr)
@@ -251,7 +262,7 @@ static int validate_constraint_nodes(sepol_handle_t *handle, unsigned int nperms
goto bad;
depth++;
- if (cexp->attr & CEXPR_XTARGET && nperms != 0)
+ if (cexp->attr & CEXPR_XTARGET && !is_validatetrans)
goto bad;
if (!(cexp->attr & CEXPR_TYPE)) {
if (validate_empty_type_set(cexp->type_names))
@@ -366,11 +377,49 @@ bad:
return -1;
}
+static int perm_visit(__attribute__((__unused__)) hashtab_key_t k, hashtab_datum_t d, void *args)
+{
+ perm_arg_t *pargs = args;
+ const perm_datum_t *perdatum = d;
+
+ if (!value_isvalid(perdatum->s.value, pargs->nprim))
+ return -1;
+
+ if (pargs->inherited_nprim != 0 && value_isvalid(perdatum->s.value, pargs->inherited_nprim))
+ return -1;
+
+ if ((UINT32_C(1) << (perdatum->s.value - 1)) & pargs->visited)
+ return -1;
+
+ pargs->visited |= (UINT32_C(1) << (perdatum->s.value - 1));
+ return 0;
+}
+
+static int validate_permission_symtab(sepol_handle_t *handle, const symtab_t *permissions, uint32_t inherited_nprim)
+{
+ /* Check each entry has a different valid value and is not overriding an inherited one */
+
+ perm_arg_t pargs = { .visited = 0, .nprim = permissions->nprim, .inherited_nprim = inherited_nprim };
+
+ if (hashtab_map(permissions->table, perm_visit, &pargs))
+ goto bad;
+
+ return 0;
+
+bad:
+ ERR(handle, "Invalid permission table");
+ return -1;
+}
+
static int validate_common_datum(sepol_handle_t *handle, const common_datum_t *common, validate_t flavors[])
{
if (validate_value(common->s.value, &flavors[SYM_COMMONS]))
goto bad;
- if (common->permissions.table->nel == 0 || common->permissions.nprim > PERM_SYMTAB_SIZE)
+ if (common->permissions.nprim == 0 || common->permissions.nprim > PERM_SYMTAB_SIZE)
+ goto bad;
+ if (common->permissions.nprim != common->permissions.table->nel)
+ goto bad;
+ if (validate_permission_symtab(handle, &common->permissions, 0))
goto bad;
return 0;
@@ -393,11 +442,17 @@ static int validate_class_datum(sepol_handle_t *handle, const class_datum_t *cla
goto bad;
if (class->comdatum && validate_common_datum(handle, class->comdatum, flavors))
goto bad;
- if (class->permissions.nprim > PERM_SYMTAB_SIZE)
+ /* empty classes are permitted */
+ if (class->permissions.nprim > PERM_SYMTAB_SIZE || class->permissions.table->nel > PERM_SYMTAB_SIZE)
+ goto bad;
+ if (class->permissions.nprim !=
+ (class->permissions.table->nel + (class->comdatum ? class->comdatum->permissions.table->nel : 0)))
+ goto bad;
+ if (validate_permission_symtab(handle, &class->permissions, class->comdatum ? class->comdatum->permissions.nprim : 0))
goto bad;
if (validate_constraint_nodes(handle, class->permissions.nprim, class->constraints, flavors))
goto bad;
- if (validate_constraint_nodes(handle, 0, class->validatetrans, flavors))
+ if (validate_constraint_nodes(handle, UINT32_MAX, class->validatetrans, flavors))
goto bad;
switch (class->default_user) {
@@ -877,46 +932,23 @@ bad:
return -1;
}
-static int perm_match(__attribute__ ((unused)) hashtab_key_t key, hashtab_datum_t datum, void *data)
-{
- const uint32_t *v = data;
- const perm_datum_t *perdatum = datum;
-
- return *v == perdatum->s.value;
-}
-
static int validate_access_vector(sepol_handle_t *handle, const policydb_t *p, sepol_security_class_t tclass,
sepol_access_vector_t av)
{
const class_datum_t *cladatum = p->class_val_to_struct[tclass - 1];
- uint32_t i;
/*
* Check that at least one permission bit is valid.
* Older compilers might set invalid bits for the wildcard permission.
*/
- for (i = 0; i < cladatum->permissions.nprim; i++) {
- if (av & (UINT32_C(1) << i)) {
- uint32_t v = i + 1;
- int rc;
-
- rc = hashtab_map(cladatum->permissions.table, perm_match, &v);
- if (rc == 1)
- goto good;
-
- if (cladatum->comdatum) {
- rc = hashtab_map(cladatum->comdatum->permissions.table, perm_match, &v);
- if (rc == 1)
- goto good;
- }
- }
- }
+ if (!(av & PERMISSION_MASK(cladatum->permissions.nprim)))
+ goto bad;
+ return 0;
+
+bad:
ERR(handle, "Invalid access vector");
return -1;
-
-good:
- return 0;
}
static int validate_avtab_key_and_datum(avtab_key_t *k, avtab_datum_t *d, void *args)
--
2.43.0
