[PATCH] selinux: tighten policydb_context_isvalid() checks

Stephen Smalley <[email protected]>
Newsgroups org.kernel.vger.selinux
Message-ID <[email protected]>
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 guars on role and
usrdatum in the OBJECT_R_VAL block can then be dropped.

Signed-off-by: Stephen Smalley <[email protected]>
---
 security/selinux/ss/policydb.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index c4a81bd31e05..3beb3bd60e0f 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -995,13 +995,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) {
@@ -1009,7 +1009,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;
 
@@ -1017,9 +1017,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;
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.