Skip to content

Commit 7bbb841

Browse files
cgzonescgzones
committed
selinux: restrict policy strings
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 <[email protected]> --- 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
1 parent 110f056 commit 7bbb841

File tree

3 files changed

+84
-23
lines changed

3 files changed

+84
-23
lines changed

‎security/selinux/ss/conditional.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp)
280280

281281
len = le32_to_cpu(buf[2]);
282282

283-
rc = str_read(&key, GFP_KERNEL, fp, len);
283+
rc = str_read_bool(&key, GFP_KERNEL, fp, len);
284284
if (rc)
285285
goto err;
286286

‎security/selinux/ss/policydb.c

+39-21
Original file line numberDiff line numberDiff line change
@@ -1226,8 +1226,9 @@ static int context_read_and_validate(struct context *c, struct policydb *p,
12261226
* binary representation file.
12271227
*/
12281228

1229-
int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
1229+
int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len, int kind, u32 max_len)
12301230
{
1231+
u32 i;
12311232
int rc;
12321233
char *str;
12331234

@@ -1237,19 +1238,35 @@ int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
12371238
if (size_check(sizeof(char), len, fp))
12381239
return -EINVAL;
12391240

1241+
if (max_len != 0 && len > max_len)
1242+
return -EINVAL;
1243+
12401244
str = kmalloc(len + 1, flags | __GFP_NOWARN);
12411245
if (!str)
12421246
return -ENOMEM;
12431247

12441248
rc = next_entry(str, fp, len);
1245-
if (rc) {
1246-
kfree(str);
1247-
return rc;
1249+
if (rc)
1250+
goto bad_str;
1251+
1252+
rc = -EINVAL;
1253+
for (i = 0; i < len; i++) {
1254+
if (iscntrl(str[i]))
1255+
goto bad_str;
1256+
1257+
if (kind == STR_IDENTIFIER &&
1258+
!(isalnum(str[i]) || str[i] == '_' || str[i] == '-' || str[i] == '.'))
1259+
goto bad_str;
1260+
12481261
}
12491262

12501263
str[len] = '\0';
12511264
*strp = str;
12521265
return 0;
1266+
1267+
bad_str:
1268+
kfree(str);
1269+
return rc;
12531270
}
12541271

12551272
static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *fp)
@@ -1274,7 +1291,7 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
12741291
if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
12751292
goto bad;
12761293

1277-
rc = str_read(&key, GFP_KERNEL, fp, len);
1294+
rc = str_read_perm(&key, GFP_KERNEL, fp, len);
12781295
if (rc)
12791296
goto bad;
12801297

@@ -1321,7 +1338,7 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
13211338
goto bad;
13221339
comdatum->permissions.nprim = le32_to_cpu(buf[2]);
13231340

1324-
rc = str_read(&key, GFP_KERNEL, fp, len);
1341+
rc = str_read_class(&key, GFP_KERNEL, fp, len);
13251342
if (rc)
13261343
goto bad;
13271344

@@ -1559,12 +1576,12 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
15591576

15601577
ncons = le32_to_cpu(buf[5]);
15611578

1562-
rc = str_read(&key, GFP_KERNEL, fp, len);
1579+
rc = str_read_class(&key, GFP_KERNEL, fp, len);
15631580
if (rc)
15641581
goto bad;
15651582

15661583
if (len2) {
1567-
rc = str_read(&cladatum->comkey, GFP_KERNEL, fp, len2);
1584+
rc = str_read_class(&cladatum->comkey, GFP_KERNEL, fp, len2);
15681585
if (rc)
15691586
goto bad;
15701587

@@ -1698,7 +1715,7 @@ static int role_read(struct policydb *p, struct symtab *s, struct policy_file *f
16981715
if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
16991716
role->bounds = le32_to_cpu(buf[2]);
17001717

1701-
rc = str_read(&key, GFP_KERNEL, fp, len);
1718+
rc = str_read_role(&key, GFP_KERNEL, fp, len);
17021719
if (rc)
17031720
goto bad;
17041721

@@ -1765,7 +1782,7 @@ static int type_read(struct policydb *p, struct symtab *s, struct policy_file *f
17651782
typdatum->primary = le32_to_cpu(buf[2]);
17661783
}
17671784

1768-
rc = str_read(&key, GFP_KERNEL, fp, len);
1785+
rc = str_read_type(&key, GFP_KERNEL, fp, len);
17691786
if (rc)
17701787
goto bad;
17711788

@@ -1829,7 +1846,7 @@ static int user_read(struct policydb *p, struct symtab *s, struct policy_file *f
18291846
if (p->policyvers >= POLICYDB_VERSION_BOUNDARY)
18301847
usrdatum->bounds = le32_to_cpu(buf[2]);
18311848

1832-
rc = str_read(&key, GFP_KERNEL, fp, len);
1849+
rc = str_read_user(&key, GFP_KERNEL, fp, len);
18331850
if (rc)
18341851
goto bad;
18351852

@@ -1878,7 +1895,7 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f
18781895
goto bad;
18791896
levdatum->isalias = val;
18801897

1881-
rc = str_read(&key, GFP_KERNEL, fp, len);
1898+
rc = str_read_sens(&key, GFP_KERNEL, fp, len);
18821899
if (rc)
18831900
goto bad;
18841901

@@ -1921,7 +1938,7 @@ static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp
19211938
goto bad;
19221939
catdatum->isalias = val;
19231940

1924-
rc = str_read(&key, GFP_KERNEL, fp, len);
1941+
rc = str_read_cat(&key, GFP_KERNEL, fp, len);
19251942
if (rc)
19261943
goto bad;
19271944

@@ -2230,7 +2247,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, struct policy_f
22302247
len = le32_to_cpu(buf[0]);
22312248

22322249
/* path component string */
2233-
rc = str_read(&name, GFP_KERNEL, fp, len);
2250+
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
22342251
if (rc)
22352252
return rc;
22362253

@@ -2329,7 +2346,7 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
23292346
len = le32_to_cpu(buf[0]);
23302347

23312348
/* path component string */
2332-
rc = str_read(&name, GFP_KERNEL, fp, len);
2349+
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
23332350
if (rc)
23342351
return rc;
23352352

@@ -2483,7 +2500,7 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
24832500
if (!newgenfs)
24842501
goto out;
24852502

2486-
rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len);
2503+
rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
24872504
if (rc)
24882505
goto out;
24892506

@@ -2522,7 +2539,7 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
25222539
if (!newc)
25232540
goto out;
25242541

2525-
rc = str_read(&newc->u.name, GFP_KERNEL, fp, len);
2542+
rc = str_read(&newc->u.name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
25262543
if (rc)
25272544
goto out;
25282545

@@ -2625,7 +2642,7 @@ static int ocontext_read(struct policydb *p,
26252642
goto out;
26262643
len = le32_to_cpu(buf[0]);
26272644

2628-
rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
2645+
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
26292646
if (rc)
26302647
goto out;
26312648

@@ -2693,7 +2710,7 @@ static int ocontext_read(struct policydb *p,
26932710
goto out;
26942711

26952712
len = le32_to_cpu(buf[1]);
2696-
rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
2713+
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
26972714
if (rc)
26982715
goto out;
26992716

@@ -2759,7 +2776,7 @@ static int ocontext_read(struct policydb *p,
27592776
len = le32_to_cpu(buf[0]);
27602777

27612778
rc = str_read(&c->u.ibendport.dev_name,
2762-
GFP_KERNEL, fp, len);
2779+
GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
27632780
if (rc)
27642781
goto out;
27652782

@@ -2827,7 +2844,8 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
28272844
goto bad;
28282845
}
28292846

2830-
rc = str_read(&policydb_str, GFP_KERNEL, fp, len);
2847+
rc = str_read(&policydb_str, GFP_KERNEL, fp, len,
2848+
STR_UNCONSTRAINT, strlen(POLICYDB_STRING));
28312849
if (rc) {
28322850
if (rc == -ENOMEM) {
28332851
pr_err("SELinux: unable to allocate memory for policydb string of length %d\n",

‎security/selinux/ss/policydb.h

+44-1
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,50 @@ static inline bool val_is_boolean(u32 value)
408408
return value == 0 || value == 1;
409409
}
410410

411-
extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len);
411+
#define STR_UNCONSTRAINT 0
412+
#define STR_IDENTIFIER 1
413+
extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len,
414+
int kind, u32 max_len);
415+
416+
static inline int str_read_bool(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
417+
{
418+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
419+
}
420+
421+
static inline int str_read_cat(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
422+
{
423+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 32);
424+
}
425+
426+
static inline int str_read_class(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
427+
{
428+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
429+
}
430+
431+
static inline int str_read_perm(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
432+
{
433+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
434+
}
435+
436+
static inline int str_read_role(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
437+
{
438+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
439+
}
440+
441+
static inline int str_read_sens(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
442+
{
443+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 32);
444+
}
445+
446+
static inline int str_read_type(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
447+
{
448+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 1024);
449+
}
450+
451+
static inline int str_read_user(char **strp, gfp_t flags, struct policy_file *fp, u32 len)
452+
{
453+
return str_read(strp, flags, fp, len, STR_IDENTIFIER, 128);
454+
}
412455

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

0 commit comments

Comments
 (0)