[userspace PATCH] Prevent free() of stack buffer with NOLOG format

[userspace PATCH] Prevent free() of stack buffer with NOLOG format

[George McCollister]

When the NOLOG format is used replace_event_msg() doesn't change
e->reply.message so the message located on the stack is left and later is
free()'d in cleanup_event() resulting in the following:

*** Error in `auditd': free(): invalid pointer: 0x800bef7c ***
======= Backtrace: =========
/lib/libc.so.6(+0x676ba)[0xb752b6ba]
/lib/libc.so.6(+0x6e227)[0xb7532227]
/lib/libc.so.6(+0x6e9e6)[0xb75329e6]
auditd(+0x73df)[0x800a43df]
auditd(+0x4975)[0x800a1975]
auditd(+0x4a9c)[0x800a1a9c]
auditd(main+0x931)[0x800a0c21]
/lib/libc.so.6(__libc_start_main+0xf6)[0xb74dc1a6]
auditd(+0x44c4)[0x800a14c4]
======= Memory map: ========
...

This patch changes the log format to RAW when NOLOG format is detected
so that replace_event_msg() will replace e->reply.message with a message
that can be free()'d by cleanup_event().

Signed-off-by: George McCollister <george.mccollister@gmail.com>
---
 src/auditd-config.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/auditd-config.c b/src/auditd-config.c
index 584f079..bc06b1c 100644
--- a/src/auditd-config.c
+++ b/src/auditd-config.c
@@ -839,6 +839,7 @@ static int log_format_parser(struct nv_pair *nv, int line,
 		if (strcasecmp(nv->value, log_formats[i].name) == 0) {
 			config->log_format = log_formats[i].option;
 			if (config->log_format == LF_NOLOG) {
+				config->log_format = LF_RAW;
 				audit_msg(LOG_WARNING,
 				    "The NOLOG option to log_format is deprecated. Please use the write_logs option.");
 				if (config->write_logs != 0)
-- 
2.9.3

Re: [userspace PATCH] Prevent free() of stack buffer with NOLOG format

[Steve Grubb]

On Monday, December 5, 2016 6:01:02 PM EST George McCollister wrote:
> When the NOLOG format is used replace_event_msg() doesn't change
> e->reply.message so the message located on the stack is left and later is
> free()'d in cleanup_event() resulting in the following:

Hmm...thanks for reporting this. Which version of audit are you using?

Steve

> *** Error in `auditd': free(): invalid pointer: 0x800bef7c ***
> ======= Backtrace: =========
> /lib/libc.so.6(+0x676ba)[0xb752b6ba]
> /lib/libc.so.6(+0x6e227)[0xb7532227]
> /lib/libc.so.6(+0x6e9e6)[0xb75329e6]
> auditd(+0x73df)[0x800a43df]
> auditd(+0x4975)[0x800a1975]
> auditd(+0x4a9c)[0x800a1a9c]
> auditd(main+0x931)[0x800a0c21]
> /lib/libc.so.6(__libc_start_main+0xf6)[0xb74dc1a6]
> auditd(+0x44c4)[0x800a14c4]
> ======= Memory map: ========
> ...
> 
> This patch changes the log format to RAW when NOLOG format is detected
> so that replace_event_msg() will replace e->reply.message with a message
> that can be free()'d by cleanup_event().
> 
> Signed-off-by: George McCollister <george.mccollister@gmail.com>
> ---
>  src/auditd-config.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/auditd-config.c b/src/auditd-config.c
> index 584f079..bc06b1c 100644
> --- a/src/auditd-config.c
> +++ b/src/auditd-config.c
> @@ -839,6 +839,7 @@ static int log_format_parser(struct nv_pair *nv, int
> line, if (strcasecmp(nv->value, log_formats[i].name) == 0) {
>  			config->log_format = log_formats[i].option;
>  			if (config->log_format == LF_NOLOG) {
> +				config->log_format = LF_RAW;
>  				audit_msg(LOG_WARNING,
>  				    "The NOLOG option to log_format is deprecated. Please use 
the
> write_logs option."); if (config->write_logs != 0)

Re: [userspace PATCH] Prevent free() of stack buffer with NOLOG format

[George McCollister]

On Mon, Dec 5, 2016 at 6:30 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> On Monday, December 5, 2016 6:01:02 PM EST George McCollister wrote:
>> When the NOLOG format is used replace_event_msg() doesn't change
>> e->reply.message so the message located on the stack is left and later is
>> free()'d in cleanup_event() resulting in the following:
>
> Hmm...thanks for reporting this. Which version of audit are you using?

I'm using 2.6.6 but I reproduced the problem and made the change
against the HEAD of the master branch (using this mirror
https://github.com/linux-audit/audit-userspace).

-George

Re: [userspace PATCH] Prevent free() of stack buffer with NOLOG format

[Steve Grubb]

On Tuesday, December 6, 2016 7:57:33 AM EST George McCollister wrote:
> On Mon, Dec 5, 2016 at 6:30 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Monday, December 5, 2016 6:01:02 PM EST George McCollister wrote:
> >> When the NOLOG format is used replace_event_msg() doesn't change
> >> e->reply.message so the message located on the stack is left and later is
> > 
> >> free()'d in cleanup_event() resulting in the following:
> > Hmm...thanks for reporting this. Which version of audit are you using?
> 
> I'm using 2.6.6 but I reproduced the problem and made the change
> against the HEAD of the master branch (using this mirror
> https://github.com/linux-audit/audit-userspace).

OK. Got it. The patch isn't exactly the right fix. While it may hide the 
problem, the intent is that people may want to use the enriched format and 
send logs to a remote collector. By any chance do you know which buffer on the 
stack is getting freed? I'm trying to reproduce this but I thought I'd ask if 
you where it is since you have already looked into it.

Thanks,
-Steve

Re: [userspace PATCH] Prevent free() of stack buffer with NOLOG format

[Steve Grubb]

On Tuesday, December 6, 2016 10:55:05 AM EST Steve Grubb wrote:
> On Tuesday, December 6, 2016 7:57:33 AM EST George McCollister wrote:
> > On Mon, Dec 5, 2016 at 6:30 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > > On Monday, December 5, 2016 6:01:02 PM EST George McCollister wrote:
> > >> When the NOLOG format is used replace_event_msg() doesn't change
> > >> e->reply.message so the message located on the stack is left and later
> > >> is
> > > 
> > >> free()'d in cleanup_event() resulting in the following:
> > > Hmm...thanks for reporting this. Which version of audit are you using?
> > 
> > I'm using 2.6.6 but I reproduced the problem and made the change
> > against the HEAD of the master branch (using this mirror
> > https://github.com/linux-audit/audit-userspace).
> 
> OK. Got it. The patch isn't exactly the right fix. While it may hide the
> problem, the intent is that people may want to use the enriched format and
> send logs to a remote collector. By any chance do you know which buffer on
> the stack is getting freed? I'm trying to reproduce this but I thought I'd
> ask if you where it is since you have already looked into it.

I committed the following patch to fix this:

https://fedorahosted.org/audit/changeset/1421

Thanks for reporting the problem!

-Steve