BUG/MEDIUM: resolvers: always check a valid item in query_list
authorWilly Tarreau <w@1wt.eu>
Tue, 19 Oct 2021 09:17:33 +0000 (11:17 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Wed, 3 Nov 2021 10:49:42 +0000 (11:49 +0100)
commit6ba9eaecb166db42efc2b555fdc46c35b39ec14d
treead849e2c6105ded6f99d7b175e1445d4a9f3b696
parent5080674951cef67b0392e1694a32b7518bed3007
BUG/MEDIUM: resolvers: always check a valid item in query_list

The query_list is physically stored in the struct resolution itself,
so we have a list that contains a list to items stored in itself (and
there is a single item). But the list is first initialized in
resolv_validate_dns_response(), while it's scanned in
resolv_process_responses() later after calling the former. First,
this results in crashes as soon as the code is instrumented a little
bit for debugging, as elements from a previous incarnation can appear.

But in addition to this, the presence of an element is checked by
verifying that the return of LIST_NEXT() is not NULL, while it may
never be NULL even for an empty list, resulting in bugs or crashes
if the number of responses does not match the list's contents. This
is easily triggered by testing for the list non-emptiness outside of
the function.

Let's make sure the list is always correct, i.e. it's initialized to
an empty list when the structure is allocated, elements are checked by
first verifying the list is not empty, they are deleted once checked,
and in any case at end so that there are no dangling pointers.

This should be backported, but only as long as the patch fits without
modifications, as adaptations can be risky there given that bugs tend
to hide each other.

(cherry picked from commit 25e010906a016c9d6e8677726e6e55de80010616)
[wt: use resolv_hostname_cmp() in 2.4]
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 51128f86129a1257b59005356253f78411d77e95)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
src/dns.c