BUG/MINOR: http-act: fix a double free of the map reference on a parsing error

parse_http_set_map() extracts the map/acl file name into <rule->arg.map.ref>,
then parses one or two log-format strings. On a parsing failure it releases the
reference before reporting the error:

	if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.key, ...)) {
		free(rule->arg.map.ref);
		return ACT_RET_PRS_ERR;
	}

but <rule->release_ptr> has already been set to release_http_map(), which starts
with "free(rule->arg.map.ref)". Since the caller calls free_act_rule() upon
ACT_RET_PRS_ERR, the same pointer is freed twice. Both error paths of the
function (the key and the value patterns) are affected, and they cover the
"add-acl", "del-acl", "set-map" and "del-map" actions.

Reproduced with:

	http-request set-map(/tmp/m.map) %[nosuchfetch] somevalue

which reports "free(): double free detected in tcache 2" and aborts, so
"haproxy -c" dies instead of reporting the configuration error.

This only happens while parsing an invalid configuration, so there is no
security impact, but a configuration checker must not crash.

Let's use ha_free() so the pointer is reset and release_http_map() becomes a
no-op for it.

This should be backported to all supported versions.

diff --git a/src/http_act.c b/src/http_act.c
index eaa1148e257fa2de5540e0b598199c524ba77d9e..d4c68e2236cec5b415fee24441ad630a9317a46e 100644 (file)
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -2356,9 +2356,11 @@ static enum act_parse_ret parse_http_set_map(const char **args, int *orig_arg, s
                        cap |= SMP_VAL_BE_HRS_HDR;
        }
 
-       /* key pattern */
+       /* key pattern. Note that <arg.map.ref> must be reset once released,
+        * otherwise release_http_map() would free it a second time.
+        */
        if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.key, LOG_OPT_NONE, cap, err)) {
-               free(rule->arg.map.ref);
+               ha_free(&rule->arg.map.ref);
                return ACT_RET_PRS_ERR;
        }
 
@@ -2366,7 +2368,7 @@ static enum act_parse_ret parse_http_set_map(const char **args, int *orig_arg, s
                /* value pattern for set-map only */
                cur_arg++;
                if (!parse_logformat_string(args[cur_arg], px, &rule->arg.map.value, LOG_OPT_NONE, cap, err)) {
-                       free(rule->arg.map.ref);
+                       ha_free(&rule->arg.map.ref);
                        return ACT_RET_PRS_ERR;
                }
        }