Skip to content

Commit ff0a443

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 35e0bb8 commit ff0a443

File tree

3 files changed

+84
-23
lines changed

3 files changed

+84
-23
lines changed

‎security/selinux/ss/conditional.c‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 39 additions & 21 deletions
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 (oom_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 > 32)
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

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

1323-
rc = str_read(&key, GFP_KERNEL, fp, len);
1340+
rc = str_read_class(&key, GFP_KERNEL, fp, len);
13241341
if (rc)
13251342
goto bad;
13261343

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

15581575
ncons = le32_to_cpu(buf[5]);
15591576

1560-
rc = str_read(&key, GFP_KERNEL, fp, len);
1577+
rc = str_read_class(&key, GFP_KERNEL, fp, len);
15611578
if (rc)
15621579
goto bad;
15631580

15641581
if (len2) {
1565-
rc = str_read(&cladatum->comkey, GFP_KERNEL, fp, len2);
1582+
rc = str_read_class(&cladatum->comkey, GFP_KERNEL, fp, len2);
15661583
if (rc)
15671584
goto bad;
15681585

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

1699-
rc = str_read(&key, GFP_KERNEL, fp, len);
1716+
rc = str_read_role(&key, GFP_KERNEL, fp, len);
17001717
if (rc)
17011718
goto bad;
17021719

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

1766-
rc = str_read(&key, GFP_KERNEL, fp, len);
1783+
rc = str_read_type(&key, GFP_KERNEL, fp, len);
17671784
if (rc)
17681785
goto bad;
17691786

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

1830-
rc = str_read(&key, GFP_KERNEL, fp, len);
1847+
rc = str_read_user(&key, GFP_KERNEL, fp, len);
18311848
if (rc)
18321849
goto bad;
18331850

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

1879-
rc = str_read(&key, GFP_KERNEL, fp, len);
1896+
rc = str_read_sens(&key, GFP_KERNEL, fp, len);
18801897
if (rc)
18811898
goto bad;
18821899

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

1922-
rc = str_read(&key, GFP_KERNEL, fp, len);
1939+
rc = str_read_cat(&key, GFP_KERNEL, fp, len);
19231940
if (rc)
19241941
goto bad;
19251942

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

22272244
/* path component string */
2228-
rc = str_read(&name, GFP_KERNEL, fp, len);
2245+
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
22292246
if (rc)
22302247
return rc;
22312248

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

23262343
/* path component string */
2327-
rc = str_read(&name, GFP_KERNEL, fp, len);
2344+
rc = str_read(&name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
23282345
if (rc)
23292346
return rc;
23302347

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

2481-
rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len);
2498+
rc = str_read(&newgenfs->fstype, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
24822499
if (rc)
24832500
goto out;
24842501

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

2520-
rc = str_read(&newc->u.name, GFP_KERNEL, fp, len);
2537+
rc = str_read(&newc->u.name, GFP_KERNEL, fp, len, STR_UNCONSTRAINT, 0);
25212538
if (rc)
25222539
goto out;
25232540

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

2623-
rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
2640+
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
26242641
if (rc)
26252642
goto out;
26262643

@@ -2688,7 +2705,7 @@ static int ocontext_read(struct policydb *p,
26882705
goto out;
26892706

26902707
len = le32_to_cpu(buf[1]);
2691-
rc = str_read(&c->u.name, GFP_KERNEL, fp, len);
2708+
rc = str_read(&c->u.name, GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
26922709
if (rc)
26932710
goto out;
26942711

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

27562773
rc = str_read(&c->u.ibendport.dev_name,
2757-
GFP_KERNEL, fp, len);
2774+
GFP_KERNEL, fp, len, STR_IDENTIFIER, 128);
27582775
if (rc)
27592776
goto out;
27602777

@@ -2822,7 +2839,8 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
28222839
goto bad;
28232840
}
28242841

2825-
rc = str_read(&policydb_str, GFP_KERNEL, fp, len);
2842+
rc = str_read(&policydb_str, GFP_KERNEL, fp, len,
2843+
STR_UNCONSTRAINT, strlen(POLICYDB_STRING));
28262844
if (rc) {
28272845
if (rc == -ENOMEM) {
28282846
pr_err("SELinux: unable to allocate memory for policydb string of length %d\n",

‎security/selinux/ss/policydb.h‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,50 @@ static inline const char *sym_name(const struct policydb *p, unsigned int sym_nu
402402
return p->sym_val_to_name[sym_num][element_nr];
403403
}
404404

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

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

0 commit comments

Comments
 (0)