From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Paris Subject: Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings Date: Thu, 11 Sep 2008 10:25:13 -0400 Message-ID: <1221143113.2992.9.camel@localhost.localdomain> References: <1221085418.2705.19.camel@amilo> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1221085418.2705.19.camel@amilo> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Miloslav =?UTF-8?Q?Trma=C4=8D?= Cc: linux-audit , viro@zeniv.linux.org.uk, linux-kernel List-Id: linux-audit@redhat.com On Thu, 2008-09-11 at 00:23 +0200, Miloslav Trma=C4=8D wrote: > From: Miloslav Trmac >=20 > 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 lengt= h > - to audit_log_n_string(), which copies the data unchanged into the > audit record. >=20 > 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. >=20 > 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