SELinux: add more validity checks on policy load

Add more validity checks at policy load time to reject malformed
policies and prevent subsequent out-of-range indexing when in permissive
mode.  Resolves the NULL pointer dereference reported in
https://bugzilla.redhat.com/show_bug.cgi?id=357541.

Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: James Morris <jmorris@namei.org>

7 files changed, 118 insertions, 38 deletions

diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index 7551af1f789..9e70a160d7d 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -325,7 +325,7 @@ static uint16_t spec_order[] = {
 	AVTAB_MEMBER
 };
 
-int avtab_read_item(void *fp, u32 vers, struct avtab *a,
+int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 	            int (*insertf)(struct avtab *a, struct avtab_key *k,
 				   struct avtab_datum *d, void *p),
 		    void *p)
@@ -333,10 +333,11 @@ int avtab_read_item(void *fp, u32 vers, struct avtab *a,
 	__le16 buf16[4];
 	u16 enabled;
 	__le32 buf32[7];
-	u32 items, items2, val;
+	u32 items, items2, val, vers = pol->policyvers;
 	struct avtab_key key;
 	struct avtab_datum datum;
 	int i, rc;
+	unsigned set;
 
 	memset(&key, 0, sizeof(struct avtab_key));
 	memset(&datum, 0, sizeof(struct avtab_datum));
@@ -420,12 +421,35 @@ int avtab_read_item(void *fp, u32 vers, struct avtab *a,
 	key.target_class = le16_to_cpu(buf16[items++]);
 	key.specified = le16_to_cpu(buf16[items++]);
 
+	if (!policydb_type_isvalid(pol, key.source_type) ||
+	    !policydb_type_isvalid(pol, key.target_type) ||
+	    !policydb_class_isvalid(pol, key.target_class)) {
+		printk(KERN_WARNING "security: avtab: invalid type or class\n");
+		return -1;
+	}
+
+	set = 0;
+	for (i = 0; i < ARRAY_SIZE(spec_order); i++) {
+		if (key.specified & spec_order[i])
+			set++;
+	}
+	if (!set || set > 1) {
+		printk(KERN_WARNING
+			"security:  avtab:  more than one specifier\n");
+		return -1;
+	}
+
 	rc = next_entry(buf32, fp, sizeof(u32));
 	if (rc < 0) {
 		printk("security: avtab: truncated entry\n");
 		return -1;
 	}
 	datum.data = le32_to_cpu(*buf32);
+	if ((key.specified & AVTAB_TYPE) &&
+	    !policydb_type_isvalid(pol, datum.data)) {
+		printk(KERN_WARNING "security: avtab: invalid type\n");
+		return -1;
+	}
 	return insertf(a, &key, &datum, p);
 }
 
@@ -435,7 +459,7 @@ static int avtab_insertf(struct avtab *a, struct avtab_key *k,
 	return avtab_insert(a, k, d);
 }
 
-int avtab_read(struct avtab *a, void *fp, u32 vers)
+int avtab_read(struct avtab *a, void *fp, struct policydb *pol)
 {
 	int rc;
 	__le32 buf[1];
@@ -459,7 +483,7 @@ int avtab_read(struct avtab *a, void *fp, u32 vers)
 		goto bad;
 
 	for (i = 0; i < nel; i++) {
-		rc = avtab_read_item(fp,vers, a, avtab_insertf, NULL);
+		rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL);
 		if (rc) {
 			if (rc == -ENOMEM)
 				printk(KERN_ERR "security: avtab: out of memory\n");
diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
index d8edf8ca56d..8da6a842808 100644
--- a/security/selinux/ss/avtab.h
+++ b/security/selinux/ss/avtab.h
@@ -64,12 +64,13 @@ struct avtab_datum *avtab_search(struct avtab *h, struct avtab_key *k);
 void avtab_destroy(struct avtab *h);
 void avtab_hash_eval(struct avtab *h, char *tag);
 
-int avtab_read_item(void *fp, uint32_t vers, struct avtab *a,
+struct policydb;
+int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol,
 		    int (*insert)(struct avtab *a, struct avtab_key *k,
 				  struct avtab_datum *d, void *p),
 		    void *p);
 
-int avtab_read(struct avtab *a, void *fp, u32 vers);
+int avtab_read(struct avtab *a, void *fp, struct policydb *pol);
 
 struct avtab_node *avtab_insert_nonunique(struct avtab *h, struct avtab_key *key,
 					  struct avtab_datum *datum);
diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 45b93a827c8..50ad85d4b77 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -362,7 +362,8 @@ static int cond_read_av_list(struct policydb *p, void *fp, struct cond_av_list *
 	data.head = NULL;
 	data.tail = NULL;
 	for (i = 0; i < len; i++) {
-		rc = avtab_read_item(fp, p->policyvers, &p->te_cond_avtab, cond_insertf, &data);
+		rc = avtab_read_item(&p->te_cond_avtab, fp, p, cond_insertf,
+				     &data);
 		if (rc)
 			return rc;
 
diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
index 9a11deaaa9e..fb5d70a6628 100644
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -157,49 +157,55 @@ void mls_sid_to_context(struct context *context,
 	return;
 }
 
+int mls_level_isvalid(struct policydb *p, struct mls_level *l)
+{
+	struct level_datum *levdatum;
+	struct ebitmap_node *node;
+	int i;
+
+	if (!l->sens || l->sens > p->p_levels.nprim)
+		return 0;
+	levdatum = hashtab_search(p->p_levels.table,
+				  p->p_sens_val_to_name[l->sens - 1]);
+	if (!levdatum)
+		return 0;
+
+	ebitmap_for_each_positive_bit(&l->cat, node, i) {
+		if (i > p->p_cats.nprim)
+			return 0;
+		if (!ebitmap_get_bit(&levdatum->level->cat, i)) {
+			/*
+			 * Category may not be associated with
+			 * sensitivity.
+			 */
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
+int mls_range_isvalid(struct policydb *p, struct mls_range *r)
+{
+	return (mls_level_isvalid(p, &r->level[0]) &&
+		mls_level_isvalid(p, &r->level[1]) &&
+		mls_level_dom(&r->level[1], &r->level[0]));
+}
+
 /*
  * Return 1 if the MLS fields in the security context
  * structure `c' are valid.  Return 0 otherwise.
  */
 int mls_context_isvalid(struct policydb *p, struct context *c)
 {
-	struct level_datum *levdatum;
 	struct user_datum *usrdatum;
-	struct ebitmap_node *node;
-	int i, l;
 
 	if (!selinux_mls_enabled)
 		return 1;
 
-	/*
-	 * MLS range validity checks: high must dominate low, low level must
-	 * be valid (category set <-> sensitivity check), and high level must
-	 * be valid (category set <-> sensitivity check)
-	 */
-	if (!mls_level_dom(&c->range.level[1], &c->range.level[0]))
-		/* High does not dominate low. */
+	if (!mls_range_isvalid(p, &c->range))
 		return 0;
 
-	for (l = 0; l < 2; l++) {
-		if (!c->range.level[l].sens || c->range.level[l].sens > p->p_levels.nprim)
-			return 0;
-		levdatum = hashtab_search(p->p_levels.table,
-			p->p_sens_val_to_name[c->range.level[l].sens - 1]);
-		if (!levdatum)
-			return 0;
-
-		ebitmap_for_each_positive_bit(&c->range.level[l].cat, node, i) {
-			if (i > p->p_cats.nprim)
-				return 0;
-			if (!ebitmap_get_bit(&levdatum->level->cat, i))
-				/*
-				 * Category may not be associated with
-				 * sensitivity in low level.
-				 */
-				return 0;
-		}
-	}
-
 	if (c->role == OBJECT_R_VAL)
 		return 1;
 
diff --git a/security/selinux/ss/mls.h b/security/selinux/ss/mls.h
index 096d1b4ef7f..ab53663d9f5 100644
--- a/security/selinux/ss/mls.h
+++ b/security/selinux/ss/mls.h
@@ -27,6 +27,8 @@
 int mls_compute_context_len(struct context *context);
 void mls_sid_to_context(struct context *context, char **scontext);
 int mls_context_isvalid(struct policydb *p, struct context *c);
+int mls_range_isvalid(struct policydb *p, struct mls_range *r);
+int mls_level_isvalid(struct policydb *p, struct mls_level *l);
 
 int mls_context_to_sid(char oldc,
 	               char **scontext,
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 539828b229b..b582aae3c62 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -713,6 +713,27 @@ out:
 	return rc;
 }
 
+int policydb_class_isvalid(struct policydb *p, unsigned int class)
+{
+	if (!class || class > p->p_classes.nprim)
+		return 0;
+	return 1;
+}
+
+int policydb_role_isvalid(struct policydb *p, unsigned int role)
+{
+	if (!role || role > p->p_roles.nprim)
+		return 0;
+	return 1;
+}
+
+int policydb_type_isvalid(struct policydb *p, unsigned int type)
+{
+	if (!type || type > p->p_types.nprim)
+		return 0;
+	return 1;
+}
+
 /*
  * Return 1 if the fields in the security context
  * structure `c' are valid.  Return 0 otherwise.
@@ -1260,6 +1281,7 @@ static int mls_read_level(struct mls_level *lp, void *fp)
 		       "categories\n");
 		goto bad;
 	}
+
 	return 0;
 
 bad:
@@ -1563,7 +1585,7 @@ int policydb_read(struct policydb *p, void *fp)
 		p->symtab[i].nprim = nprim;
 	}
 
-	rc = avtab_read(&p->te_avtab, fp, p->policyvers);
+	rc = avtab_read(&p->te_avtab, fp, p);
 	if (rc)
 		goto bad;
 
@@ -1595,6 +1617,12 @@ int policydb_read(struct policydb *p, void *fp)
 		tr->role = le32_to_cpu(buf[0]);
 		tr->type = le32_to_cpu(buf[1]);
 		tr->new_role = le32_to_cpu(buf[2]);
+		if (!policydb_role_isvalid(p, tr->role) ||
+		    !policydb_type_isvalid(p, tr->type) ||
+		    !policydb_role_isvalid(p, tr->new_role)) {
+			rc = -EINVAL;
+			goto bad;
+		}
 		ltr = tr;
 	}
 
@@ -1619,6 +1647,11 @@ int policydb_read(struct policydb *p, void *fp)
 			goto bad;
 		ra->role = le32_to_cpu(buf[0]);
 		ra->new_role = le32_to_cpu(buf[1]);
+		if (!policydb_role_isvalid(p, ra->role) ||
+		    !policydb_role_isvalid(p, ra->new_role)) {
+			rc = -EINVAL;
+			goto bad;
+		}
 		lra = ra;
 	}
 
@@ -1872,9 +1905,19 @@ int policydb_read(struct policydb *p, void *fp)
 				rt->target_class = le32_to_cpu(buf[0]);
 			} else
 				rt->target_class = SECCLASS_PROCESS;
+			if (!policydb_type_isvalid(p, rt->source_type) ||
+			    !policydb_type_isvalid(p, rt->target_type) ||
+			    !policydb_class_isvalid(p, rt->target_class)) {
+				rc = -EINVAL;
+				goto bad;
+			}
 			rc = mls_read_range_helper(&rt->target_range, fp);
 			if (rc)
 				goto bad;
+			if (!mls_range_isvalid(p, &rt->target_range)) {
+				printk(KERN_WARNING "security:  rangetrans:  invalid range\n");
+				goto bad;
+			}
 			lrt = rt;
 		}
 	}
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 844d310f4f1..ed6fc687c66 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -251,6 +251,9 @@ struct policydb {
 extern void policydb_destroy(struct policydb *p);
 extern int policydb_load_isids(struct policydb *p, struct sidtab *s);
 extern int policydb_context_isvalid(struct policydb *p, struct context *c);
+extern int policydb_class_isvalid(struct policydb *p, unsigned int class);
+extern int policydb_type_isvalid(struct policydb *p, unsigned int type);
+extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
 extern int policydb_read(struct policydb *p, void *fp);
 
 #define PERM_SYMTAB_SIZE 32