Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings
On Thu, 2008-09-11 at 00:23 +0200, Miloslav Trmač wrote:
From: Miloslav Trmac <mitr(a)redhat.com>
audit_string_contains_control() stops checking at the first NUL byte.
If audit_string_contains_control() returns FALSE,
audit_log_n_untrustedstring() submits the complete string - including
the NUL byte and all following bytes, up to the specified maximum length
- to audit_log_n_string(), which copies the data unchanged into the
audit record.
The audit record can thus contain a NUL byte (and some unchecked data
after that). Because the user-space audit daemon treats audit records
as NUL-terminated strings, an untrusted string that is shorter than the
specified maximum length effectively terminates the audit record.
This patch modifies audit_log_n_untrustedstring() to only log the data
before the first NUL byte, if any.
I'm going to have to say NAK on this patch.
It's still not right looking at the other user,
audit_log_single_execve_arg(). An execve arg with a NULL could loose
the stuff after the NULL (not break the record like audit_tty) since the
execve uses %s rather than calling trusted string.
How about we change the meaning of audit_string_contains_control()
return values? If it returns positive that is the number of bytes in a
legitimate string up to the first null. -1 means it is hex. That
eliminates your code duplication and allows us to do the right thing in
both the generic untrusted_string code and the execve code.....
-Eric