selinux: tighten policydb_context_isvalid() checks

policydb_context_isvalid() bounds each of user, role, and type but
does not require the value to be defined. A crafted policy can declare
more identifiers that it defines, so an initial SID or ocontext
context whose type is not defined can be inserted into the sidtab and
later reach type_attribute_bounds_av() from
context_struct_compute_av(), thereby reaching the
BUG_ON(!type_val_to_struct[type - 1]). A user with no datum can reach
context_struct_to_string() and thereby trigger strlen(NULL). The user
and role datums are already NULL-checked further down but only when
role is not OBJECT_R_VAL, so an object context can carry an undefined
value through.

Use policydb_{role,user,type}_isvalid(), which check both the range
and that the value has a name, in place of the inline range checks.
The name and datum arrays are populated by *_index(), so a value with
a name also has a datum. This runs after policydb_index(), so the
arrays are populated. The now-redundant NULL guards on role and
usrdatum in the OBJECT_R_VAL block can then be dropped.

Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

1 files changed, 4 insertions, 7 deletions

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index b75c977c6673..4342258cb148 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -1037,13 +1037,13 @@ bool policydb_context_isvalid(const struct policydb *p, const struct context *c)
 	const struct role_datum *role;
 	const struct user_datum *usrdatum;
 
-	if (!c->role || c->role > p->p_roles.nprim)
+	if (!policydb_role_isvalid(p, c->role))
 		return false;
 
-	if (!c->user || c->user > p->p_users.nprim)
+	if (!policydb_user_isvalid(p, c->user))
 		return false;
 
-	if (!c->type || c->type > p->p_types.nprim)
+	if (!policydb_simpletype_isvalid(p, c->type))
 		return false;
 
 	if (c->role != OBJECT_R_VAL) {
@@ -1051,7 +1051,7 @@ bool policydb_context_isvalid(const struct policydb *p, const struct context *c)
 		 * Role must be authorized for the type.
 		 */
 		role = p->role_val_to_struct[c->role - 1];
-		if (!role || !ebitmap_get_bit(&role->types, c->type - 1))
+		if (!ebitmap_get_bit(&role->types, c->type - 1))
 			/* role may not be associated with type */
 			return false;
 
@@ -1059,9 +1059,6 @@ bool policydb_context_isvalid(const struct policydb *p, const struct context *c)
 		 * User must be authorized for the role.
 		 */
 		usrdatum = p->user_val_to_struct[c->user - 1];
-		if (!usrdatum)
-			return false;
-
 		if (!ebitmap_get_bit(&usrdatum->roles, c->role - 1))
 			/* user may not be associated with role */
 			return false;