Skip to content

Commit

Permalink
selinux: restrict policy strings
Browse files Browse the repository at this point in the history 
Validate the characters and the lengths of strings parsed from binary
policies.

  * Disallow control characters
  * Limit characters of identifiers to alphanumeric, underscore, dash,
    and dot
  * Limit identifiers in length to 128, expect types to 1024 and
    categories to 32, characters (excluding NUL-terminator)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
  - add wrappers for str_read() to minimize the usage of magic numbers
  - limit sensitivities to a length of 32, to match categories,
    suggested by Daniel
  • Loading branch information
@cgzones
cgzones committed Jan 7, 2025
1 parent a1c6ee0 commit 6a550ad
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 23 deletions.
2 changes: 1 addition & 1 deletion security/selinux/ss/conditional.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp)

len = le32_to_cpu(buf[2]);

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_bool(&key, GFP_KERNEL, fp, len);
if (rc)
goto err;

Expand Down
60 changes: 39 additions & 21 deletions security/selinux/ss/policydb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,8 +1226,9 @@ static int context_read_and_validate(struct context *c, struct policydb *p,
* binary representation file.
*/

int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len, int kind, u32 max_len)
{
u32 i;
int rc;
char *str;

Expand All @@ -1237,19 +1238,35 @@ int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
if (oom_check(sizeof(char), len, fp))
return -EINVAL;

if (max_len != 0 && len > max_len)
return -EINVAL;

str = kmalloc(len + 1, flags | __GFP_NOWARN);
if (!str)
return -ENOMEM;

rc = next_entry(str, fp, len);
if (rc) {
kfree(str);
return rc;
if (rc)
goto bad_str;

rc = -EINVAL;
for (i = 0; i < len; i++) {
if (iscntrl(str[i]))
goto bad_str;

if (kind == STR_IDENTIFIER &&
!(isalnum(str[i]) || str[i] == '_' || str[i] == '-' || str[i] == '.'))
goto bad_str;

}

str[len] = '\0';
*strp = str;
return 0;

bad_str:
kfree(str);
return rc;
}

static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *fp)
Expand All @@ -1274,7 +1291,7 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
if (perdatum->value < 1 || perdatum->value > 32)
goto bad;

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_perm(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1320,7 +1337,7 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
goto bad;
comdatum->permissions.nprim = le32_to_cpu(buf[2]);

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_class(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1557,12 +1574,12 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *

ncons = le32_to_cpu(buf[5]);

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_class(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

if (len2) {
rc = str_read(&cladatum->comkey, GFP_KERNEL, fp, len2);
rc = str_read_class(&cladatum->comkey, GFP_KERNEL, fp, len2);
if (rc)
goto bad;

Expand Down Expand Up @@ -1696,7 +1713,7 @@ static int role_read(struct policydb *p, struct symtab *s, struct policy_file *f
if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
role->bounds = le32_to_cpu(buf[2]);

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_role(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1763,7 +1780,7 @@ static int type_read(struct policydb *p, struct symtab *s, struct policy_file *f
typdatum->primary = le32_to_cpu(buf[2]);
}

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_type(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1827,7 +1844,7 @@ static int user_read(struct policydb *p, struct symtab *s, struct policy_file *f
if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
usrdatum->bounds = le32_to_cpu(buf[2]);

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_user(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1876,7 +1893,7 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f
goto bad;
levdatum->isalias = val;

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_sens(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -1919,7 +1936,7 @@ static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp
goto bad;
catdatum->isalias = val;

rc = str_read(&key, GFP_KERNEL, fp, len);
rc = str_read_cat(&key, GFP_KERNEL, fp, len);
if (rc)
goto bad;

Expand Down Expand Up @@ -2225,7 +2242,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, struct policy_f
len = le32_to_cpu(buf[0]);

/* path component string */
rc = str_read(&name, GFP_KERNEL, fp, len);
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
if (rc)
return rc;

Expand Down Expand Up @@ -2324,7 +2341,7 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
len = le32_to_cpu(buf[0]);

/* path component string */
rc = str_read(&name, GFP_KERNEL, fp, len);
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
if (rc)
return rc;

Expand Down Expand Up @@ -2478,7 +2495,7 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
if (!newgenfs)
goto out;

rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len);
rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
if (rc)
goto out;

Expand Down Expand Up @@ -2517,7 +2534,7 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
if (!newc)
goto out;

rc = str_read(&newc->u.name, GFP_KERNEL, fp, len);
rc = str_read(&newc->u.name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
if (rc)
goto out;

Expand Down Expand Up @@ -2620,7 +2637,7 @@ static int ocontext_read(struct policydb *p,
goto out;
len = le32_to_cpu(buf[0]);

rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
if (rc)
goto out;

Expand Down Expand Up @@ -2688,7 +2705,7 @@ static int ocontext_read(struct policydb *p,
goto out;

len = le32_to_cpu(buf[1]);
rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
if (rc)
goto out;

Expand Down Expand Up @@ -2754,7 +2771,7 @@ static int ocontext_read(struct policydb *p,
len = le32_to_cpu(buf[0]);

rc = str_read(&c->u.ibendport.dev_name,
GFP_KERNEL, fp, len);
GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
if (rc)
goto out;

Expand Down Expand Up @@ -2822,7 +2839,8 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
goto bad;
}

rc = str_read(&policydb_str, GFP_KERNEL, fp, len);
rc = str_read(&policydb_str, GFP_KERNEL, fp, len,
STR_UNCONSTRAINT, strlen(POLICYDB_STRING));
if (rc) {
if (rc == -ENOMEM) {
pr_err("SELinux: unable to allocate memory for policydb string of length %d\n",
Expand Down
45 changes: 44 additions & 1 deletion security/selinux/ss/policydb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,50 @@ static inline const char *sym_name(const struct policydb *p, unsigned int sym_nu
return p->sym_val_to_name[sym_num][element_nr];
}

extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len);
#define STR_UNCONSTRAINT 0
#define STR_IDENTIFIER 1
extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len,
int kind, u32 max_len);

static inline int str_read_bool(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
}

static inline int str_read_cat(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 32);
}

static inline int str_read_class(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
}

static inline int str_read_perm(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
}

static inline int str_read_role(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
}

static inline int str_read_sens(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 32);
}

static inline int str_read_type(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 1024);
}

static inline int str_read_user(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
{
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
}

extern u16 string_to_security_class(struct policydb *p, const char *name);
extern u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name);
Expand Down

0 comments on commit 6a550ad

Please sign in to comment.