BUG/MINOR: ssl/ckch: fix an invalid free() in the domains/ips error paths
The "domains" and "ips" keywords are stored as NULL-terminated arrays of
strings. When an allocation fails while one of these arrays is built,
both ckch_conf_parse() and ckchs_dup() release it with:
while (r && *r) {
char *prev = *r;
r++;
free(prev);
}
free(r);
The loop advances r to the terminating NULL entry, so free() is called
on r + n instead of the start of the array. When the failing my_strndup()
or strdup() is not the first one, glibc aborts the process with
"free(): invalid pointer" (ASan reports "attempting free on address
which was not malloc()-ed").
ckch_conf_parse() is reached from the configuration ("crt-store",
"crt-list") and from "add ssl crt-list" on the CLI. ckchs_dup() is
reached at runtime from "set ssl cert" on the CLI, from Lua
CertCache.set() and from the ACME scheduler when a certificate with at
least two domains or IPs is renewed, so the whole process is killed. In
both cases an allocation failure is needed.
Fix it by freeing the entries by index and then the array itself,
leaving r unchanged. ha_freearray() cannot be used here since it does
not free the array.
The issue was reproduced by failing the malloc() of the second domain
with an LD_PRELOAD shim: without the patch both paths abort, with it
"haproxy -c" reports "out of memory" and the CLI answers "Can't update
@s/site.pem!" while the process keeps running.
The ckch_conf_parse() part was introduced by commit
2e8acf54d ("BUG/MINOR:
ssl/ckch: leak in error path") and the ckchs_dup() part by commit
5b85b81d8 ("MINOR: ssl/ckch: handle ckch_conf in ckchs_dup() and
ckch_conf_clean()"), both in 3.2-dev, so this must be backported to 3.2.
This fixes GitHub issue #3561.