Blob Blame History Raw
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