From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Dennis Subject: Re: [PATCH 1/2] audit: fix NUL handling in untrusted strings Date: Thu, 11 Sep 2008 15:47:33 -0400 Message-ID: <48C975D5.3020603@redhat.com> References: <1221085418.2705.19.camel@amilo> <1221143113.2992.9.camel@localhost.localdomain> <48C955C8.2000602@redhat.com> <1221156612.17533.14.camel@amilo> <48C96D90.70608@redhat.com> <1221161260.17533.30.camel@amilo> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1085694224==" Return-path: In-Reply-To: <1221161260.17533.30.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: =?UTF-8?B?TWlsb3NsYXYgVHJtYcSN?= Cc: linux-audit , viro@zeniv.linux.org.uk, linux-kernel List-Id: linux-audit@redhat.com This is a multi-part message in MIME format. --===============1085694224== Content-Type: multipart/alternative; boundary="------------040607000604040107080107" This is a multi-part message in MIME format. --------------040607000604040107080107 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Miloslav Trma=C4=8D wrote: > If the interface says "NUL-terminated string", any bytes after that are > not "actual data". Yes, that's correct. However, the function in question,=20 audit_log_n_untrustedstring() is not an interface accepting a null=20 terminated string, it accepts a count. The helper function on which it=20 is dependent, audit_string_contains_control(), disregards the length=20 parameter it is passed and thus audit_log_n_untrustedstring() misbehaves=20 as a consequence. >> It would be wrong for the audit system to assume the memory block it >> was pointed to only ever contained null terminated ascii strings, >> especially when the memory block is terminated by virtue of an octet >> count. >> =20 > Yes, that's why it was wrong to use audit_*string() for TTY input data. > And the 2/2 patch fixes it - at the source of the problem, not in an > unrelated function that was incorrectly used. > =20 This is true, but it's only part of the problem, the string functions=20 still need to be robust, even used inappropriately. --=20 John Dennis --------------040607000604040107080107 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Miloslav Trma=C4=8D wrote:

If the interface says "NUL-terminated string", any bytes=
 after that are
not "actual data".

Yes, that's correct. However, the function in question, audit_log_n_untrustedstring() is not an interface accepting a null terminated string, it accepts a count. The helper function on which it is dependent, audit_string_contains_control(), disregards the length parameter it is passed and thus audit_log_n_untrustedstring() misbehaves as a consequence.

It would be wrong for the audit system to assume the m=
emory block it
was pointed to only ever contained null terminated ascii strings,
especially when the memory block is terminated by virtue of an octet
count.

Yes, that's why it was wrong to use audit_*string=
() for TTY input data.
And the 2/2 patch fixes it - at the source of the problem, not in an
unrelated function that was incorrectly used.

This is true, but it's only part of the problem, the string functions still need to be robust, even used inappropriately.

--=20
John Dennis <jdennis@redhat.com>

--------------040607000604040107080107-- --===============1085694224== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============1085694224==--