selinux: more strict bounds check

Validate the types used in bounds checks.
Replace the usage of BUG(), to avoid halting the system on malformed
polices.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
[PM: merge fuzz, fixed typo identified by Smalley]
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 8e00305af1e9be79967a50dfb0cec279da6f1a36..5c7fec936d2992970a241da0db02c4adb3eb391c 100644 (file)
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -937,6 +937,15 @@ bool policydb_class_isvalid(const struct policydb *p, u16 class)
        return true;
 }
 
+bool policydb_user_isvalid(const struct policydb *p, u32 user)
+{
+       if (!user || user > p->p_users.nprim)
+               return false;
+       if (!p->sym_val_to_name[SYM_USERS][user - 1])
+               return false;
+       return true;
+}
+
 bool policydb_role_isvalid(const struct policydb *p, u32 role)
 {
        if (!role || role > p->p_roles.nprim)
@@ -1785,6 +1794,12 @@ static int user_bounds_sanity_check(void *key, void *datum, void *datap)
                        return -EINVAL;
                }
 
+               if (!policydb_user_isvalid(p, upper->bounds)) {
+                       pr_err("SELinux: user %s: invalid boundary id %d\n",
+                              (char *) key, upper->bounds);
+                       return -EINVAL;
+               }
+
                upper = p->user_val_to_struct[upper->bounds - 1];
                ebitmap_for_each_positive_bit(&user->roles, node, bit)
                {
@@ -1822,6 +1837,12 @@ static int role_bounds_sanity_check(void *key, void *datum, void *datap)
                        return -EINVAL;
                }
 
+               if (!policydb_role_isvalid(p, upper->bounds)) {
+                       pr_err("SELinux: role %s: invalid boundary id %d\n",
+                              (char *) key, upper->bounds);
+                       return -EINVAL;
+               }
+
                upper = p->role_val_to_struct[upper->bounds - 1];
                ebitmap_for_each_positive_bit(&role->types, node, bit)
                {
@@ -1856,9 +1877,13 @@ static int type_bounds_sanity_check(void *key, void *datum, void *datap)
                        return -EINVAL;
                }
 
-               upper = p->type_val_to_struct[upper->bounds - 1];
-               BUG_ON(!upper);
+               if (!policydb_type_isvalid(p, upper->bounds)) {
+                       pr_err("SELinux: type %s: invalid boundary id %d\n",
+                              (char *) key, upper->bounds);
+                       return -EINVAL;
+               }
 
+               upper = p->type_val_to_struct[upper->bounds - 1];
                if (upper->attribute) {
                        pr_err("SELinux: type %s: "
                               "bounded by attribute %s\n",

diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 85633116623988346122f66aa63f459dc84cea1e..b1a64c23e1dcd7bde89faa31094be2bf0542e098 100644 (file)
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -327,6 +327,7 @@ extern bool policydb_context_isvalid(const struct policydb *p,
 extern bool policydb_class_isvalid(const struct policydb *p, u16 class);
 extern bool policydb_type_isvalid(const struct policydb *p, u32 type);
 extern bool policydb_role_isvalid(const struct policydb *p, u32 role);
+extern bool policydb_user_isvalid(const struct policydb *p, u32 user);
 extern int policydb_read(struct policydb *p, struct policy_file *fp);
 extern int policydb_write(struct policydb *p, struct policy_file *fp);
 

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index d3d6d6ea556b8ea6fbd2b8a06d3dce8b5da60cc2..bfa2ab2557305175902686a7a1dd99b8f8bb45c6 100644 (file)
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -715,6 +715,9 @@ static void context_struct_compute_av(struct policydb *policydb,
         * If the given source and target types have boundary
         * constraint, lazy checks have to mask any violated
         * permission and notice it to userspace via audit.
+        *
+        * Infinite recursion is avoided via a depth pre-check in
+        * type_bounds_sanity_check().
         */
        type_attribute_bounds_av(policydb, scontext, tcontext,
                                 tclass, avd);