From: Valentine Krasnobaeva Date: Mon, 12 Aug 2024 13:32:00 +0000 (+0200) Subject: BUG/MINOR: pattern: pat_ref_set: fix UAF reported by coverity X-Git-Tag: v3.1-dev6~51 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4f2493f355;p=thirdparty%2Fhaproxy.git BUG/MINOR: pattern: pat_ref_set: fix UAF reported by coverity memprintf() performs realloc and updates then the pointer to an output buffer, where it has written the data. So free() is called on the previous buffer address, if it was provided. pat_ref_set_elt() uses memprintf() to write its error message as well as pat_ref_set(). So, when we re-enter into the while loop the second time and pat_ref_set_elt() has returned, the *err ptr (previous value of *merr) is already freed by memprintf() from pat_ref_set_el(). 'if (!found)' condition is false at this point, because we've found a node at the first loop. So, the second memprintf(), in order to write error messages, does again free(*err). This should be backported in all stable versions. --- diff --git a/src/pattern.c b/src/pattern.c index f07223f7c3..da81e17513 100644 --- a/src/pattern.c +++ b/src/pattern.c @@ -1774,17 +1774,8 @@ int pat_ref_set_by_id(struct pat_ref *ref, struct pat_ref_elt *refelt, const cha int pat_ref_set(struct pat_ref *ref, const char *key, const char *value, char **err, struct pat_ref_elt *elt) { int found = 0; - char *_merr; - char **merr; struct ebmb_node *node; - if (err) { - merr = &_merr; - *merr = NULL; - } - else - merr = NULL; - if (elt) { node = &elt->node; } @@ -1794,17 +1785,13 @@ int pat_ref_set(struct pat_ref *ref, const char *key, const char *value, char ** } while (node) { + char *tmp_err = NULL; + elt = ebmb_entry(node, struct pat_ref_elt, node); node = ebmb_next_dup(node); - if (!pat_ref_set_elt(ref, elt, value, merr)) { - if (err && merr) { - if (!found) { - *err = *merr; - } else { - memprintf(err, "%s, %s", *err, *merr); - ha_free(merr); - } - } + if (!pat_ref_set_elt(ref, elt, value, &tmp_err)) { + memprintf(err, "%s, %s", err && *err ? *err : "", tmp_err); + ha_free(&tmp_err); } found = 1; }