Skip to content

Commit a065bde

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 8d86c3a commit a065bde

File tree

2 files changed

+36
-7
lines changed

2 files changed

+36
-7
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: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1247,6 +1247,7 @@ static int context_struct_to_string(struct policydb *p,
12471247
char **scontext, u32 *scontext_len)
12481248
{
12491249
char *scontextp;
1250+
int mls_len;
12501251

12511252
if (scontext)
12521253
*scontext = NULL;
@@ -1266,11 +1267,33 @@ static int context_struct_to_string(struct policydb *p,
12661267
*scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1;
12671268
*scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1;
12681269
*scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
1269-
*scontext_len += mls_compute_context_len(p, context);
1270+
1271+
mls_len = mls_compute_context_len(p, context);
1272+
if (unlikely(mls_len < 0)) {
1273+
pr_warn_ratelimited("SELinux: %s: MLS security context component too large ([%d:%d]-[%d:%d])",
1274+
__func__,
1275+
context->range.level[0].sens,
1276+
ebitmap_length(&context->range.level[0].cat),
1277+
context->range.level[1].sens,
1278+
ebitmap_length(&context->range.level[1].cat));
1279+
return -EOVERFLOW;
1280+
}
1281+
*scontext_len += mls_len;
12701282

12711283
if (!scontext)
12721284
return 0;
12731285

1286+
if (unlikely(*scontext_len > CONTEXT_MAXLENGTH)) {
1287+
pr_warn_ratelimited("SELinux: %s: security context string of length %d too large (%d:%d:%d:[%d:%d]-[%d:%d])",
1288+
__func__, *scontext_len,
1289+
context->user, context->role, context->type,
1290+
context->range.level[0].sens,
1291+
ebitmap_length(&context->range.level[0].cat),
1292+
context->range.level[1].sens,
1293+
ebitmap_length(&context->range.level[1].cat));
1294+
return -EOVERFLOW;
1295+
}
1296+
12741297
/* Allocate space for the context; caller must free this space. */
12751298
scontextp = kmalloc(*scontext_len, GFP_ATOMIC);
12761299
if (!scontextp)

0 commit comments

Comments
 (0)