Messages in this thread | | | Subject | Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings | From | Miloslav Trmač <> | Date | Thu, 11 Sep 2008 20:10:12 +0200 |
| |
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
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |