Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings
John Dennis píše v Čt 11. 09. 2008 v 13:30 -0400:
Special processing with regards to the presence or absence of a null
byte is one example of prohibited interpretation.
This is UNIX, "string"
means "NUL-terminated string" (in fact the
presence of a NUL byte is the only way to reasonably detect binary
data).
You're far more likely to encounter a fixed-length field with an
optional terminating NUL (like the old-style, 16-byte directory entries)
than an ASCII-compatible string that intentionally contains a NUL byte.
TTY input auditing was the only place where it makes a difference, all
other code was passing a string that was at least as long as the
specified size to audit_log_n_untrustedstring().
It seems to me the problem is with audit_string_contains_control():
int audit_string_contains_control(const char *string, size_t len)
{
const unsigned char *p;
for (p = string; p < (const unsigned char *)string + len && *p; p
++) {
if (*p == '"' || *p < 0x21 || *p > 0x7e)
return 1;
}
return 0;
}
The problem is that it is passed a counted octet sequence but in some
circumstances ignores the count. This occurs when *p == 0, the test
for NULL should be removed. If that test is removed it will return the
flag which indicates the string must be encoded differently to be
conformant with the protocol.
Yes, that's possible - but then
audit_log_n_untrustedstring() would be
more accurately called audit_log_n_ascii_like_binary_data().
Anyway, Eric/Al - if you prefer this solution, I can prepare an
alternative patch.
As a side note I'm concerned there may be places in the user
audit
code which treat string data as null terminated (at least that is my
recollection).
Yes, auditd adds a NUL terminator to the audit record, and then
treats
it as a regular NUL-terminated string; if the audit record contains an
embedded NUL byte, the rest of the record is discarded by auditd.
Mirek