From 96a197fb642933b625996947b2e18ff2ed1df261 Mon Sep 17 00:00:00 2001 From: Steve Grubb 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