auparse: refact nvlist cleanup code

Resolves: rhbz#2008965

0005-auparse-refact-nvlist-cleanup-cod.patch

@@ -0,0 +1,142 @@
+From 96a197fb642933b625996947b2e18ff2ed1df261 Mon Sep 17 00:00:00 2001
+From: Steve Grubb <sgrubb@redhat.com>
+Date: Wed, 3 Nov 2021 10:22:55 -0300
+Subject: [PATCH 5/5] auparse: refact nvlist cleanup cod
+
+The nvlist_clear function tries to guess whether we allocated a buffer
+or if the pointer points to a spot in the parsed up record by using a
+name. This is fragile because someone actually used a reserved name in
+a record unexpectedly. The refactored code does a range check to see if
+the pointer is within the parsed up buffer area. If not, it can free the
+buffer.
+
+Backport of upstream 5baa2a860c
+---
+ auparse/ellist.c | 12 ++++++++++--
+ auparse/nvlist.c | 23 +++++++++++++++++------
+ auparse/nvlist.h |  2 +-
+ auparse/rnode.h  |  1 +
+ 4 files changed, 29 insertions(+), 9 deletions(-)
+
+diff --git a/auparse/ellist.c b/auparse/ellist.c
+index bbb4064..048ac4c 100644
+--- a/auparse/ellist.c
++++ b/auparse/ellist.c
+@@ -103,7 +103,7 @@ static char *escape(const char *tmp)
+ static int parse_up_record(rnode* r)
+ {
+ 	char *ptr, *buf, *saved=NULL;
+-	unsigned int offset = 0;
++	unsigned int offset = 0, len;
+ 
+ 	// Potentially cut the record in two
+ 	ptr = strchr(r->record, AUDIT_INTERP_SEPARATOR);
+@@ -112,7 +112,15 @@ static int parse_up_record(rnode* r)
+ 		ptr++;
+ 	}
+ 	r->interp = ptr;
+-	r->nv.record = buf = strdup(r->record);
++	// Rather than call strndup, we will do it ourselves to reduce
++	// the number of interations across the record.
++	// len includes the string terminator.
++	len = strlen(r->record) + 1;
++	r->nv.record = buf = malloc(len);
++	if (r->nv.record == NULL)
++		return -1;
++	memcpy(r->nv.record, r->record, len);
++	r->nv.end = r->nv.record + len;
+ 	ptr = audit_strsplit_r(buf, &saved);
+ 	if (ptr == NULL) {
+ 		free(buf);
+diff --git a/auparse/nvlist.c b/auparse/nvlist.c
+index 99c7c2b..c9ccc8f 100644
+--- a/auparse/nvlist.c
++++ b/auparse/nvlist.c
+@@ -36,11 +36,13 @@ void nvlist_create(nvlist *l)
+ 		l->cur = 0;
+ 		l->cnt = 0;
+ 		l->record = NULL;
++		l->end = NULL;
+ 	}
+ }
+ 
+ nvnode *nvlist_next(nvlist *l)
+ {
++	// Since cur will be incremented, check for 1 less that total
+ 	if (l->cnt && l->cur < (l->cnt - 1)) {
+ 		l->cur++;
+ 		return &l->array[l->cur];
+@@ -125,11 +127,21 @@ const char *nvlist_interp_cur_val(rnode *r, auparse_esc_t escape_mode)
+ 	return do_interpret(r, escape_mode);
+ }
+ 
++// This function determines if a chunk of memory is part of the parsed up
++// record. If it is, do not free it since it gets free'd at the very end.
++// NOTE: This function causes invalid-pointer-pair errors with ASAN
++static inline int not_in_rec_buf(nvlist *l, const char *ptr)
++{
++	if (ptr >= l->record && ptr < l->end)
++		return 0;
++	return 1;
++}
++
+ // free_interp does not apply to thing coming from interpretation_list
+-void nvlist_clear(nvlist* l, int free_interp)
++void nvlist_clear(nvlist *l, int free_interp)
+ {
+ 	unsigned int i = 0;
+-	register nvnode* current;
++	register nvnode *current;
+ 
+ 	if (l->cnt == 0)
+ 		return;
+@@ -140,11 +152,9 @@ void nvlist_clear(nvlist* l, int free_interp)
+ 			free(current->interp_val);
+ 			// A couple items are not in parsed up list.
+ 			// These all come from the aup_list_append path.
+-			if ((strcmp(current->name, "key") == 0) ||
+-			    (strcmp(current->name, "seperms") == 0) ||
+-			    (strcmp(current->name, "seresult") == 0)) {
++			if (not_in_rec_buf(l, current->name)) {
+ 				// seperms & key values are strdup'ed
+-				if (current->name[2] != 'r')
++				if (not_in_rec_buf(l, current->val))
+ 					free(current->val);
+ 				free(current->name);
+ 			}
+@@ -153,6 +163,7 @@ void nvlist_clear(nvlist* l, int free_interp)
+ 	}
+ 	free((void *)l->record);
+ 	l->record = NULL;
++	l->end = NULL;
+ 	l->cur = 0;
+ 	l->cnt = 0;
+ }
+diff --git a/auparse/nvlist.h b/auparse/nvlist.h
+index b006caa..59f3e13 100644
+--- a/auparse/nvlist.h
++++ b/auparse/nvlist.h
+@@ -45,7 +45,7 @@ static inline const char *nvlist_get_cur_val_interp(nvlist *l)
+ AUDIT_HIDDEN_START
+ 
+ void nvlist_create(nvlist *l);
+-void nvlist_clear(nvlist* l, int free_interp);
++void nvlist_clear(nvlist *l, int free_interp);
+ nvnode *nvlist_next(nvlist *l);
+ int nvlist_get_cur_type(rnode *r);
+ const char *nvlist_interp_cur_val(rnode *r, auparse_esc_t escape_mode);
+diff --git a/auparse/rnode.h b/auparse/rnode.h
+index 06303c7..69f0843 100644
+--- a/auparse/rnode.h
++++ b/auparse/rnode.h
+@@ -40,6 +40,7 @@ typedef struct {
+   unsigned int cur;     // Index to current node
+   unsigned int cnt;     // How many items in this list
+   char *record;		// Holds the parsed up record
++  char *end;		// End of the parsed up record
+ } nvlist;
+ 
+ 
+-- 
+2.33.1
+

audit.spec

@@ -2,7 +2,7 @@
 Summary: User space tools for kernel auditing
 Name: audit
 Version: 3.0.5
-Release: 4%{?dist}
+Release: 5%{?dist}
 License: GPLv2+
 URL: http://people.redhat.com/sgrubb/audit/
 Source0: http://people.redhat.com/sgrubb/audit/%{name}-%{version}.tar.gz
@@ -12,6 +12,7 @@ Patch1: 0001-Add-ausysrulevalidate.patch
 Patch2: 0002-audit-3.0.6-time.patch
 Patch3: 0003-Carry-functions-file-from-initscripts-in-audit.patch
 Patch4: 0004-When-interpreting-if-val-is-NULL-return-an-empty-str.patch
+Patch5: 0005-auparse-refact-nvlist-cleanup-cod.patch
 
 BuildRequires: make gcc swig
 BuildRequires: openldap-devel
@@ -96,6 +97,7 @@ cp %{SOURCE1} .
 %patch2 -p1
 %patch3 -p1
 %patch4 -p1
+%patch5 -p1
 
 # Remove the ids code, its not ready
 sed -i 's/ ids / /' audisp/plugins/Makefile.in
@@ -266,6 +268,10 @@ fi
 %attr(750,root,root) %{_sbindir}/audispd-zos-remote
 
 %changelog
+* Wed Nov 03 2021 Sergio Correia <scorreia@redhat.com> - 3.0.5-5
+- auparse: refact nvlist cleanup code
+  Resolves: rhbz#2008965
+
 * Wed Nov 03 2021 Sergio Correia <scorreia@redhat.com> - 3.0.5-4
 - When interpreting, if val is NULL return an empty string
   Resolves: rhbz#2004420