Skip to content

Commit

Permalink
SELinux: add more validity checks on policy load
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Stephen Smalley authored and James Morris committed Nov 7, 2007
1 parent 6d2b685 commit 45e5421
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 38 deletions.
32 changes: 28 additions & 4 deletions security/selinux/ss/avtab.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,19 @@ 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)
{
__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));
Expand Down Expand Up @@ -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);
}

Expand All @@ -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];
Expand All @@ -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");
Expand Down
5 changes: 3 additions & 2 deletions security/selinux/ss/avtab.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion security/selinux/ss/conditional.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
66 changes: 36 additions & 30 deletions security/selinux/ss/mls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions security/selinux/ss/mls.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
45 changes: 44 additions & 1 deletion security/selinux/ss/policydb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,27 @@ int policydb_load_isids(struct policydb *p, struct sidtab *s)
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.
Expand Down Expand Up @@ -1260,6 +1281,7 @@ static int mls_read_level(struct mls_level *lp, void *fp)
"categories\n");
goto bad;
}

return 0;

bad:
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
Expand Down
3 changes: 3 additions & 0 deletions security/selinux/ss/policydb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 45e5421

Please sign in to comment.