Skip to content

Commit 2f32d51

Browse files
committed
selinux: harden MLS context string generation against overflows
Check the length accumulator for the MLS component of security contexts does not overflow in mls_compute_context_len() resulting in out-of-buffer writes in mls_sid_to_context(). Signed-off-by: Christian Göttsche <[email protected]> --- v3: add patch
1 parent 7a0c566 commit 2f32d51

File tree

2 files changed

+49
-12
lines changed

2 files changed

+49
-12
lines changed

security/selinux/ss/mls.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ int mls_compute_context_len(struct policydb *p, struct context *context)
4242
len = 1; /* for the beginning ":" */
4343
for (l = 0; l < 2; l++) {
4444
u32 index_sens = context->range.level[l].sens;
45-
len += strlen(sym_name(p, SYM_LEVELS, index_sens - 1));
45+
if (check_add_overflow(len, strlen(sym_name(p, SYM_LEVELS, index_sens - 1)), &len))
46+
return -EOVERFLOW;
4647

4748
/* categories */
4849
head = -2;
@@ -54,24 +55,29 @@ int mls_compute_context_len(struct policydb *p, struct context *context)
5455
/* one or more negative bits are skipped */
5556
if (head != prev) {
5657
nm = sym_name(p, SYM_CATS, prev);
57-
len += strlen(nm) + 1;
58+
if (check_add_overflow(len, strlen(nm) + 1, &len))
59+
return -EOVERFLOW;
5860
}
5961
nm = sym_name(p, SYM_CATS, i);
60-
len += strlen(nm) + 1;
62+
if (check_add_overflow(len, strlen(nm) + 1, &len))
63+
return -EOVERFLOW;
6164
head = i;
6265
}
6366
prev = i;
6467
}
6568
if (prev != head) {
6669
nm = sym_name(p, SYM_CATS, prev);
67-
len += strlen(nm) + 1;
70+
if (check_add_overflow(len, strlen(nm) + 1, &len))
71+
return -EOVERFLOW;
6872
}
6973
if (l == 0) {
7074
if (mls_level_eq(&context->range.level[0],
7175
&context->range.level[1]))
7276
break;
73-
else
74-
len++;
77+
else {
78+
if (check_add_overflow(len, 1, &len))
79+
return -EOVERFLOW;
80+
}
7581
}
7682
}
7783

security/selinux/ss/services.c

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,10 +1247,12 @@ static int context_struct_to_string(struct policydb *p,
12471247
char **scontext, u32 *scontext_len)
12481248
{
12491249
char *scontextp;
1250+
size_t len;
1251+
int mls_len;
12501252

12511253
if (scontext)
12521254
*scontext = NULL;
1253-
*scontext_len = 0;
1255+
len = 0;
12541256

12551257
if (context->len) {
12561258
*scontext_len = context->len;
@@ -1263,16 +1265,45 @@ static int context_struct_to_string(struct policydb *p,
12631265
}
12641266

12651267
/* Compute the size of the context. */
1266-
*scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1;
1267-
*scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1;
1268-
*scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
1269-
*scontext_len += mls_compute_context_len(p, context);
1268+
len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1;
1269+
len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1;
1270+
len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
1271+
1272+
mls_len = mls_compute_context_len(p, context);
1273+
if (unlikely(mls_len < 0)) {
1274+
pr_warn_ratelimited("SELinux: %s: MLS security context component too large [%s:%s:%s[:[%s:%d]-[%s:%d]]]",
1275+
__func__,
1276+
sym_name(p, SYM_USERS, context->user - 1),
1277+
sym_name(p, SYM_ROLES, context->role - 1),
1278+
sym_name(p, SYM_TYPES, context->type - 1),
1279+
sym_name(p, SYM_LEVELS, context->range.level[0].sens - 1),
1280+
ebitmap_length(&context->range.level[0].cat),
1281+
sym_name(p, SYM_LEVELS, context->range.level[1].sens - 1),
1282+
ebitmap_length(&context->range.level[1].cat));
1283+
return -EOVERFLOW;
1284+
}
1285+
1286+
if (unlikely(check_add_overflow(len, mls_len, &len) || len > CONTEXT_MAXLENGTH)) {
1287+
pr_warn_ratelimited("SELinux: %s: security context string of length %zu too large [%s:%s:%s[:[%s:%d]-[%s:%d]]]",
1288+
__func__,
1289+
len,
1290+
sym_name(p, SYM_USERS, context->user - 1),
1291+
sym_name(p, SYM_ROLES, context->role - 1),
1292+
sym_name(p, SYM_TYPES, context->type - 1),
1293+
sym_name(p, SYM_LEVELS, context->range.level[0].sens - 1),
1294+
ebitmap_length(&context->range.level[0].cat),
1295+
sym_name(p, SYM_LEVELS, context->range.level[1].sens - 1),
1296+
ebitmap_length(&context->range.level[1].cat));
1297+
return -EOVERFLOW;
1298+
}
1299+
1300+
*scontext_len = len;
12701301

12711302
if (!scontext)
12721303
return 0;
12731304

12741305
/* Allocate space for the context; caller must free this space. */
1275-
scontextp = kmalloc(*scontext_len, GFP_ATOMIC);
1306+
scontextp = kmalloc(len, GFP_ATOMIC);
12761307
if (!scontextp)
12771308
return -ENOMEM;
12781309
*scontext = scontextp;

0 commit comments

Comments
 (0)