From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 6 Jan 2025 07:41:57 +0000 (+0100)
Subject: BUG/MEDIUM: promex/resolvers: Don't dump metrics if no nameserver is defined
X-Git-Tag: v3.2-dev3~10
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=892eb2bb2ceba2c0e80d51eed69dfc9494a56988;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: promex/resolvers: Don't dump metrics if no nameserver is defined

A 'resolvers' section may be defined without any nameserver. In that case,
we must take care to not dump corresponding Prometheus metrics. However
there is an issue that could lead to a crash or a strange infinite loop
because we are looping on an empty list and, at some point, we are
dereferencing an invalid pointer.

There is an issue because the loop on the nameservers of a resolvers section
is performed via callback functions and not the standard list_for_each_entry
macro. So we must take care to properly detect end of the list and empty
lists for nameservers. But the fix is not so simple because resolvers
sections with and without nameservers may be mixed.

To fix the issue, in rslv_promex_start_ts() and rslv_promex_next_ts(), when the
next resolvers section must be evaluated, a loop is now used to properly skip
empty sections.

This patch is related to #2831. Not sure it fixes it. It must be backported
as far as 3.0.
---

diff --git a/src/resolvers.c b/src/resolvers.c
index e1550cf126..d97e580561 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -3959,8 +3959,13 @@ static void *rslv_promex_start_ts(void *unused, unsigned int id)
 	if (LIST_ISEMPTY(&sec_resolvers))
 		return NULL;
 
-	resolver = LIST_NEXT(&sec_resolvers, struct resolvers *, list);
-	return LIST_NEXT(&resolver->nameservers, struct dns_nameserver *, list);
+	/* Find the first resolvers with at least one nameserver */
+	list_for_each_entry(resolver, &sec_resolvers, list) {
+		if (LIST_ISEMPTY(&resolver->nameservers))
+			continue;
+		return LIST_NEXT(&resolver->nameservers, struct dns_nameserver *, list);
+	}
+	return NULL;
 }
 
 static void *rslv_promex_next_ts(void *unused, void *metric_ctx, unsigned int id)
@@ -3970,10 +3975,14 @@ static void *rslv_promex_next_ts(void *unused, void *metric_ctx, unsigned int id
 
 	ns = LIST_NEXT(&ns->list, struct dns_nameserver *, list);
 	if (&ns->list == &resolver->nameservers) {
+		/* Find the next resolver with at least on nameserver */
 		resolver = LIST_NEXT(&resolver->list, struct resolvers *, list);
-		ns = ((&resolver->list == &sec_resolvers)
-		      ? NULL
-		      : LIST_NEXT(&resolver->nameservers, struct dns_nameserver *, list));
+		list_for_each_entry_from(resolver, &sec_resolvers, list) {
+			if (LIST_ISEMPTY(&resolver->nameservers))
+				continue;
+			return LIST_NEXT(&resolver->nameservers, struct dns_nameserver *, list);
+		}
+		ns = NULL;
 	}
 	return ns;
 }