All instances of one field type should be encoded in the same way. Since some invalid_context fields can contain untrusted strings, encode all instances of this field the same way. Please see github issue https://github.com/linux-audit/audit-kernel/issues/57 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- Passes audit-testsuite. security/selinux/ss/services.c | 48 +++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index cc043bc8fd4c..817576802f7d 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1588,6 +1588,8 @@ static int compute_sid_handle_invalid_context( struct policydb *policydb = &state->ss->policydb; char *s = NULL, *t = NULL, *n = NULL; u32 slen, tlen, nlen; + struct audit_buffer *ab; + size_t audit_size; if (context_struct_to_string(policydb, scontext, &s, &slen)) goto out; @@ -1595,12 +1597,22 @@ static int compute_sid_handle_invalid_context( goto out; if (context_struct_to_string(policydb, newcontext, &n, &nlen)) goto out; - audit_log(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR, - "op=security_compute_sid invalid_context=%s" - " scontext=%s" - " tcontext=%s" - " tclass=%s", - n, s, t, sym_name(policydb, SYM_CLASSES, tclass-1)); + /* We strip a nul only if it is at the end, otherwise the + * context contains a nul and we should audit that */ + if (n) { + if (n[nlen - 1] == '\0') + audit_size = nlen - 1; + else + audit_size = nlen; + } else { + audit_size = 0; + } + ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR); + audit_log_format(ab, "op=security_compute_sid invalid_context="); + audit_log_n_untrustedstring(ab, n, audit_size); + audit_log_format(ab, " scontext=%s tcontext=%s tclass=%s", + s, t, sym_name(policydb, SYM_CLASSES, tclass-1)); + audit_log_end(ab); out: kfree(s); kfree(t); @@ -3007,10 +3019,26 @@ int security_sid_mls_copy(struct selinux_state *state, if (rc) { if (!context_struct_to_string(policydb, &newcon, &s, &len)) { - audit_log(audit_context(), - GFP_ATOMIC, AUDIT_SELINUX_ERR, - "op=security_sid_mls_copy " - "invalid_context=%s", s); + struct audit_buffer *ab; + size_t audit_size; + + /* We strip a nul only if it is at the + * end, otherwise the context contains a + * nul and we should audit that */ + if (s) { + if (s[len - 1] == '\0') + audit_size = len - 1; + else + audit_size = len; + } else { + audit_size = 0; + } + ab = audit_log_start(audit_context(), + GFP_ATOMIC, + AUDIT_SELINUX_ERR); + audit_log_format(ab, "op=security_sid_mls_copy invalid_context="); + audit_log_n_untrustedstring(ab, s, audit_size); + audit_log_end(ab); kfree(s); } goto out_unlock; -- 1.8.3.1