From: Eric Paris <eparis@redhat.com>
To: linux-audit@redhat.com, linux-kernel@vger.kernel.org
Cc: mitr@redhat.com, akpm@linux-foundation.org, viro@zeniv.linux.org.uk
Subject: [PATCH] Audit: fix handling of 'strings' with NULL characters
Date: Thu, 11 Sep 2008 17:48:39 -0400 [thread overview]
Message-ID: <1221169719.2952.14.camel@localhost.localdomain> (raw)
currently audit_log_n_untrustedstring() uses
audit_string_contains_control() to check if the 'string' has any control
characters. If the 'string' has an embedded NULL
audit_string_contains_control() will return that the data has no control
characters and will then pass the string to audit_log_n_string with the
total length, not the length up to the first NULL. audit_log_n_string
does a memcpy of the entire length and so the actual audit record
emitted may then contain a NULL and then whatever random memory is after
the NULL.
Since we want to log the entire octet stream (if we can't trust the data
to be a string we can't trust that a NULL isn't actually a part of it)
we should just consider NULL as a control character. If the caller is
certain they want to stop at the first NULL they should be using
audit_log_untrustedstring.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
Miloslav, this is also going to take care of nulls in the TTY_AUDIT_USER
message from userspace. Is it going to be common to have control
characters on that code path as well? Do you want to change
audit_receive_msg() to also use the hex encoding directly instead of the
_n_untrustedstring interface?
kernel/audit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 4414e93..ccb8d68 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1370,7 +1370,7 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string,
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++) {
+ for (p = string; p < (const unsigned char *)string + len; p++) {
if (*p == '"' || *p < 0x21 || *p > 0x7e)
return 1;
}
WARNING: multiple messages have this Message-ID (diff)
From: Eric Paris <eparis@redhat.com>
To: linux-audit@redhat.com, linux-kernel@vger.kernel.org
Cc: viro@zeniv.linux.org.uk, jdennis@redhat.com, mitr@redhat.com,
akpm@linux-foundation.org, sgrubb@redhat.com
Subject: [PATCH] Audit: fix handling of 'strings' with NULL characters
Date: Thu, 11 Sep 2008 17:48:39 -0400 [thread overview]
Message-ID: <1221169719.2952.14.camel@localhost.localdomain> (raw)
currently audit_log_n_untrustedstring() uses
audit_string_contains_control() to check if the 'string' has any control
characters. If the 'string' has an embedded NULL
audit_string_contains_control() will return that the data has no control
characters and will then pass the string to audit_log_n_string with the
total length, not the length up to the first NULL. audit_log_n_string
does a memcpy of the entire length and so the actual audit record
emitted may then contain a NULL and then whatever random memory is after
the NULL.
Since we want to log the entire octet stream (if we can't trust the data
to be a string we can't trust that a NULL isn't actually a part of it)
we should just consider NULL as a control character. If the caller is
certain they want to stop at the first NULL they should be using
audit_log_untrustedstring.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
Miloslav, this is also going to take care of nulls in the TTY_AUDIT_USER
message from userspace. Is it going to be common to have control
characters on that code path as well? Do you want to change
audit_receive_msg() to also use the hex encoding directly instead of the
_n_untrustedstring interface?
kernel/audit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/audit.c b/kernel/audit.c
index 4414e93..ccb8d68 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1370,7 +1370,7 @@ void audit_log_n_string(struct audit_buffer *ab, const char *string,
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++) {
+ for (p = string; p < (const unsigned char *)string + len; p++) {
if (*p == '"' || *p < 0x21 || *p > 0x7e)
return 1;
}
next reply other threads:[~2008-09-11 21:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-11 21:48 Eric Paris [this message]
2008-09-11 21:48 ` [PATCH] Audit: fix handling of 'strings' with NULL characters Eric Paris
2008-09-12 0:09 ` Andrew Morton
2008-09-12 0:09 ` Andrew Morton
2008-09-12 20:03 ` Miloslav Trmač
2008-09-12 20:03 ` Miloslav Trmač
2008-09-12 19:59 ` [PATCH] Audit: Ignore terminating NUL in AUDIT_USER_TTY messages Miloslav Trmač
2008-09-12 19:59 ` Miloslav Trmač
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1221169719.2952.14.camel@localhost.localdomain \
--to=eparis@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-audit@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mitr@redhat.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.