There is a bug on parsing file path in auditd-config.c and audispd-pconfig.c

There is a bug on parsing file path in auditd-config.c and audispd-pconfig.c

[wangf]

Hi Steve,

There is a bug in function dispatch_parser() and path_parser().

when we use dir = dirname (tdir), if tdir is not NULL, tdir and dir 
point to the same addr., so if we use free(tdir) before 
audit_msg(LOG_ERR, "The directory name: %s is too short - line %d", dir, 
line); we can not get the dir's correct value.

This patch can solve this problem.

Signed-off-by: Wang Fang <wangf@cn.fujitsu.com>
---
diff -Nrup audit-1.7.4/audisp/audispd-pconfig.c audit-1.7.4-new/audisp/audispd-pconfig.c
--- audit-1.7.4/audisp/audispd-pconfig.c	2007-09-02 23:24:15.000000000 +0800
+++ audit-1.7.4-new/audisp/audispd-pconfig.c	2008-06-21 18:33:14.000000000 +0800
@@ -379,10 +379,10 @@ static int path_parser(struct nv_pair *n
 	if (tdir)
 		dir = dirname(tdir);
 	if (dir == NULL || strlen(dir) < 4) { //  '/var' is shortest dirname
-		free(tdir);
 		audit_msg(LOG_ERR,
 			"The directory name: %s is too short - line %d",
 			dir, line);
+		free(tdir);
 		return 1;
 	}
 
diff -Nrup audit-1.7.4/src/auditd-config.c audit-1.7.4-new/src/auditd-config.c
--- audit-1.7.4/src/auditd-config.c	2008-05-09 22:44:38.000000000 +0800
+++ audit-1.7.4-new/src/auditd-config.c	2008-06-21 18:39:58.000000000 +0800
@@ -592,10 +592,10 @@ static int dispatch_parser(struct nv_pai
 	if (tdir)
 		dir = dirname(tdir);
 	if (dir == NULL || strlen(dir) < 4) { //  '/var' is shortest dirname
-		free(tdir);
 		audit_msg(LOG_ERR,
 			"The directory name: %s is too short - line %d",
 			dir, line);
+		free(tdir);
 		return 1;
 	}
 
--
Best Regards,
Wang Fang

Re: There is a bug on parsing file path in auditd-config.c and audispd-pconfig.c

[wangf]

Hi Steve,

What do think about this patch?
When user defines path's dirname's length < 4bit in 
/etc/audit/auditd.conf or in plugin configure file,this error always 
happens.
If you have other opinion, please tell me, thank you.

-----Original Message-----
From: wangf
Sent: 2008-7-1 14:43
> Hi Steve,
>
> There is a bug in function dispatch_parser() and path_parser().
>
> when we use dir = dirname (tdir), if tdir is not NULL, tdir and dir 
> point to the same addr., so if we use free(tdir) before 
> audit_msg(LOG_ERR, "The directory name: %s is too short - line %d", 
> dir, line); we can not get the dir's correct value.

Because dir's space has been freed, so the space can be used by others.
when we get dir's value, it will be wrong.
Please look the two examples:

1)
define "dispatcher = /my/audispd" in /etc/audit/auditd.conf
This error message will be printed into syslog such as:
"Jul 11 10:39:04 localhost auditd: The directory name: /etc/localtime is 
too short - line 12"

but in fact it should be such error message:
"Jul 11 10:39:04 localhost auditd: The directory name: /my is too short 
- line 12"

2)
The same problem in plugin configure file:
define "path = /my/myplugin" in /etc/audisp/plugins.d/user.conf

This error message will be printed into syslog such as:
"Jul 11 10:38:39 localhost audispd: The directory name: 8óô·^C is too 
short - line 3"

but in fact it should be such error message:
"Jul 11 10:38:39 localhost audispd: The directory name: /my is too short 
- line 3"

>
> This patch can solve this problem.
>
> Signed-off-by: Wang Fang <wangf@cn.fujitsu.com>
> ---
> diff -Nrup audit-1.7.4/audisp/audispd-pconfig.c 
> audit-1.7.4-new/audisp/audispd-pconfig.c
> --- audit-1.7.4/audisp/audispd-pconfig.c    2007-09-02 
> 23:24:15.000000000 +0800
> +++ audit-1.7.4-new/audisp/audispd-pconfig.c    2008-06-21 
> 18:33:14.000000000 +0800
> @@ -379,10 +379,10 @@ static int path_parser(struct nv_pair *n
>     if (tdir)
>         dir = dirname(tdir);
>     if (dir == NULL || strlen(dir) < 4) { //  '/var' is shortest dirname
> -        free(tdir);
>         audit_msg(LOG_ERR,
>             "The directory name: %s is too short - line %d",
>             dir, line);
> +        free(tdir);
>         return 1;
>     }
>
> diff -Nrup audit-1.7.4/src/auditd-config.c 
> audit-1.7.4-new/src/auditd-config.c
> --- audit-1.7.4/src/auditd-config.c    2008-05-09 22:44:38.000000000 
> +0800
> +++ audit-1.7.4-new/src/auditd-config.c    2008-06-21 
> 18:39:58.000000000 +0800
> @@ -592,10 +592,10 @@ static int dispatch_parser(struct nv_pai
>     if (tdir)
>         dir = dirname(tdir);
>     if (dir == NULL || strlen(dir) < 4) { //  '/var' is shortest dirname
> -        free(tdir);
>         audit_msg(LOG_ERR,
>             "The directory name: %s is too short - line %d",
>             dir, line);
> +        free(tdir);
>         return 1;
>     }
>
> -- 
> Best Regards,
> Wang Fang
>
>
>
> -- 
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>
>

Re: There is a bug on parsing file path in auditd-config.c and audispd-pconfig.c

[Steve Grubb]

On Thursday 24 July 2008 00:12:51 wangf wrote:
> What do think about this patch?

This looks good, too.

Thanks,
-Steve