BUG/MINOR: acl: fix a possible arg corruption in smp_fetch_acl_parse()

smp_fetch_acl_parse() first places the newly allocated ACL sample into
the first argument to be parsed, *before* parsing it. The type is not
changed so the first argument remains of type string. In case of error,
the allocated sample is released and release_sample_expr() will call
release_sample_arg() to release the argument, possibly freeing the
string present there. And here's the catch: by overwriting the first
arguments's ->ptr entry, it happens to be located over the ->str.size
location, to not be null and to still be freed, but by pure chance
thanks to aliasing. A slight reorder of the args or buffer fields
could place it in the ->area and provoke a double-free, or even always
make the first argument's parsing fail.

Let's move the assignment after the loop has succeeded instead, and
properly set type=ARGT_PTR so that we never try to free it. The bug
appeared with the "acl()" sample fetch in 2.9 with commit 7fccccccea
("MINOR: acl: add acl() sample fetch") so it can be backported to 3.0.

diff --git a/src/acl.c b/src/acl.c
index 08538f9dffd559283b9764a3eccdfed1a59a03f3..5950da97df9c30753b81511c77242a446b806535 100644 (file)
--- a/src/acl.c
+++ b/src/acl.c
@@ -1364,8 +1364,6 @@ int smp_fetch_acl_parse(struct arg *args, char **err_msg)
        LIST_APPEND(&acl_sample->cond.suites, &acl_sample->suite.list);
        acl_sample->cond.val = ~0U; // the keyword is valid everywhere for now.
 
-       args->data.ptr = acl_sample;
-
        for (i = 0; args[i].type != ARGT_STOP; i++) {
                name = args[i].data.str.area;
                if (name[0] == '!') {
@@ -1388,6 +1386,9 @@ int smp_fetch_acl_parse(struct arg *args, char **err_msg)
                LIST_APPEND(&acl_sample->suite.terms, &acl_sample->terms[i].list);
        }
 
+       /* make the argument for smp_fetch_acl() */
+       args->data.ptr = acl_sample;
+       args->type     = ARGT_PTR;
        return 1;
 
 err: