c-ares: backport security fixes from 1.34.9

Add the eight unique upstream commits that fix the security advisories addressed in c-ares 1.34.9.
Several advisories share commits, so patches are ordered by commit rather than duplicated per advisory; each patch lists the GHSA IDs it resolves.

Assisted-by: opencode (big-pickle)
Signed-off-by: Jason Schonberg <schonm@gmail.com>
This commit is contained in:
Jason Schonberg authored and Ankur Tyagi committed 2026-10-27 17:43:29 +13:00
1 parent d3e07213ff
commit 0d9abd6842
9 files changed
+6525

No files matched your search

@@ -0,0 +1,393 @@
From f944c4718fd672c9c0befd65a340abb2ccfc1a31 Mon Sep 17 00:00:00 2001
From: Brad House <brad@brad-house.com>
Date: Sun, 18 Oct 2026 20:28:11 -0400
Subject: [PATCH] Deliver deferred socket/server-state notifications only from
the outermost public call (#1340)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Backport of f023a52a (#1340) to v1.34.
The recent change that queues socket-state and server-state
notifications delivers them, and runs the deferred cleanup, only from
the outermost `ares_deferred_cb_leave()`. `ares_send_nolock()` brackets
`ares_send_query()` itself, but most public lookup functions never
opened a region of their own. When reached from `ares_getaddrinfo()`,
`ares_query_dnsrec()` and similar, the bracket inside
`ares_send_nolock()` was therefore the outermost one. Notifications were
then delivered from inside an internal call while the library was still
in the middle of the public call, and its callers were still using their
lookup state (the host query, the query id out-pointer, ...). A
server-state callback that reconfigures the servers, which
`ares_set_server_state_callback(3)` documents as allowed, could end that
lookup before the caller was done with it.
This is a regression in unreleased code. No released version is
affected.
The outermost region is now opened by the public entry points
themselves, under the channel lock:
- `ares_send_dnsrec()` (also used by `ares_send()`)
- `ares_query_dnsrec()` (also used by `ares_query()`)
- `ares_search()`, `ares_search_dnsrec()`
- `ares_getaddrinfo()` (also used by `ares_gethostbyname()`)
- `ares_gethostbyaddr()`, `ares_getnameinfo()`
- `ares_process_pending_write()`: a failed flush closes the connection
and fails its queries over, and this function had no bracket at all.
`ares_process_fds()` (and through it
`ares_process()`/`ares_process_fd()`), `ares_cancel()`, `ares_destroy()`
and the server configuration functions already had their own. The nested
bracket in `ares_send_nolock()` stays, because it keeps connections and
servers unlinked during the send readable until the send returns.
Internal sends made from query callbacks (search next domain,
getaddrinfo next lookup, requeue, server probes) already run inside the
processing bracket.
The comments on `ares_deferred_cb_enter()` and the flush now state the
rule: the outermost region belongs to the public entry point, never to
an internal helper.
The documented behaviour (notifications arrive before the outermost
c-ares call returns) does not change, so no man page changes.
Two new `LibraryTest` cases in `test/ares-test-mock.cc`. The IPv6 server
cannot get a socket, so the first send fails over to the IPv4 server,
and a server-state callback then reconfigures the servers:
- `ServerFailoverDuringGetAddrInfo`: the lookup completes cleanly. It
fails under ASAN without this change.
- `ServerFailoverDuringQuery`: the query id from `ares_query_dnsrec()`
is already reported by the time the query callback runs. It fails
without this change.
The full non-live suite passes under ASAN/UBSAN and with
`-DCARES_WERROR=ON`.
Signed-off-by: Brad House <brad@brad-house.com>
Upstream-Status: Backport [https://github.com/c-ares/c-ares/commit/f944c4718fd672c9c0befd65a340abb2ccfc1a31]
GHSA: GHSA-wpr3-8q3r-4q8c, GHSA-97vq-4mxv-7vhm, GHSA-mw23-pfg6-2x9r, GHSA-rf6g-ww74-97qh, GHSA-v33j-65h4-4ch8, GHSA-pj7p-v3cw-hq66
---
src/lib/ares_conn.c | 4 +-
src/lib/ares_getaddrinfo.c | 5 ++
src/lib/ares_gethostbyaddr.c | 5 ++
src/lib/ares_getnameinfo.c | 5 ++
src/lib/ares_private.h | 10 ++++
src/lib/ares_process.c | 6 ++
src/lib/ares_query.c | 5 ++
src/lib/ares_search.c | 10 ++++
src/lib/ares_send.c | 13 ++++-
test/ares-test-mock.cc | 106 +++++++++++++++++++++++++++++++++++
10 files changed, 166 insertions(+), 3 deletions(-)
diff --git a/src/lib/ares_conn.c b/src/lib/ares_conn.c
index 78f57e7d10..4cc0d1871d 100644
--- a/src/lib/ares_conn.c
+++ b/src/lib/ares_conn.c
@@ -94,7 +94,9 @@ static void ares_deferred_cb_flush(ares_channel_t *channel)
{
/* Only the outermost frame delivers: anything above us on the stack may
* still be holding the connection a reentrant call from one of these
- * callbacks could free.
+ * callbacks could free, or lookup state of its own that a callback could
+ * end. That is why the outermost frame belongs to the public entry point
+ * and never to an internal helper; see ares_deferred_cb_enter().
*
* The dispatch below counts as a nesting level of its own. Without that, a
* callback that calls back into the library re-enters here through the
diff --git a/src/lib/ares_getaddrinfo.c b/src/lib/ares_getaddrinfo.c
index 6d9e21af4b..5d9ddb311e 100644
--- a/src/lib/ares_getaddrinfo.c
+++ b/src/lib/ares_getaddrinfo.c
@@ -762,7 +762,12 @@ void ares_getaddrinfo(ares_channel_t *channel, const char *name,
return;
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
ares_getaddrinfo_int(channel, name, service, hints, callback, arg);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
}
diff --git a/src/lib/ares_gethostbyaddr.c b/src/lib/ares_gethostbyaddr.c
index 69c509ab11..b314359331 100644
--- a/src/lib/ares_gethostbyaddr.c
+++ b/src/lib/ares_gethostbyaddr.c
@@ -112,7 +112,12 @@ void ares_gethostbyaddr(ares_channel_t *channel, const void *addr, int addrlen,
return;
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
ares_gethostbyaddr_nolock(channel, addr, addrlen, family, callback, arg);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
}
diff --git a/src/lib/ares_getnameinfo.c b/src/lib/ares_getnameinfo.c
index 0159354530..7365e9ed82 100644
--- a/src/lib/ares_getnameinfo.c
+++ b/src/lib/ares_getnameinfo.c
@@ -194,7 +194,12 @@ void ares_getnameinfo(ares_channel_t *channel, const struct sockaddr *sa,
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
ares_getnameinfo_int(channel, sa, salen, flags_int, callback, arg);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
}
diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h
index 0a1029e0f8..9610ba7dab 100644
--- a/src/lib/ares_private.h
+++ b/src/lib/ares_private.h
@@ -676,6 +676,16 @@ void ares_server_reap(ares_server_t *server);
* Nesting is counted; queued notifications are delivered by the outermost
* leave.
*
+ * The outermost region must be opened by the public entry point itself
+ * (ares_send_dnsrec(), ares_query_dnsrec(), ares_search(), ares_getaddrinfo(),
+ * ares_process_fds(), ares_set_servers_*(), ...), with the channel lock
+ * held, around everything it does. Internal helpers such as
+ * ares_send_nolock() open nested regions of their own, but their callers
+ * often keep using lookup state afterwards -- a host or search query, a
+ * query id out-pointer -- that a callback reentering the library may end
+ * and free. If such a helper were the outermost region, its leave would
+ * run the callbacks while that state is still in use.
+ *
* \param[in] channel Initialized ares channel.
*/
void ares_deferred_cb_enter(ares_channel_t *channel);
diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c
index 82f53610fb..1569408b1b 100644
--- a/src/lib/ares_process.c
+++ b/src/lib/ares_process.c
@@ -490,6 +490,11 @@ void ares_process_pending_write(ares_channel_t *channel)
return;
}
+ /* A failed flush closes the connection and fails its queries over, which
+ * notifies. Deliver once we are done walking the servers, and keep
+ * anything unlinked meanwhile readable until then. */
+ ares_deferred_cb_enter(channel);
+
/* Set as untriggerd before calling into ares_conn_flush(), this is
* because its possible ares_conn_flush() might cause additional data to
* be enqueued if there is some form of exception so it will need to recurse.
@@ -513,6 +518,7 @@ void ares_process_pending_write(ares_channel_t *channel)
}
}
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
}
diff --git a/src/lib/ares_query.c b/src/lib/ares_query.c
index ca3b6a9b73..0cf7f70760 100644
--- a/src/lib/ares_query.c
+++ b/src/lib/ares_query.c
@@ -125,7 +125,12 @@ ares_status_t ares_query_dnsrec(ares_channel_t *channel, const char *name,
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
status = ares_query_nolock(channel, name, dnsclass, type, callback, arg, qid);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
return status;
}
diff --git a/src/lib/ares_search.c b/src/lib/ares_search.c
index c605caf42c..ebc3d91e20 100644
--- a/src/lib/ares_search.c
+++ b/src/lib/ares_search.c
@@ -463,7 +463,12 @@ void ares_search(ares_channel_t *channel, const char *name, int dnsclass,
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
ares_search_int(channel, dnsrec, ares_dnsrec_convert_cb, carg);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
ares_dns_record_destroy(dnsrec);
@@ -481,7 +486,12 @@ ares_status_t ares_search_dnsrec(ares_channel_t *channel,
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
status = ares_search_int(channel, dnsrec, callback, arg);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
return status;
diff --git a/src/lib/ares_send.c b/src/lib/ares_send.c
index 2e0482bf19..dd4a28544b 100644
--- a/src/lib/ares_send.c
+++ b/src/lib/ares_send.c
@@ -246,8 +246,12 @@ ares_status_t ares_send_nolock(ares_channel_t *channel, ares_server_t *server,
/* Perform the first query action. */
/* Sending opens and writes to connections and can mark a server down, both
- * of which notify. Deliver those once we are done with the connection and
- * the server, not part-way through. */
+ * of which notify. This bracket keeps any connection or server unlinked
+ * during the send readable until it returns. It is never the outermost
+ * one: every public entry point that can get here, and query processing,
+ * holds its own, so delivery waits until the caller -- which may still be
+ * holding its own lookup state, as ares_getaddrinfo() does -- has unwound
+ * too. */
ares_deferred_cb_enter(channel);
status = ares_send_query(server, query, &now);
ares_deferred_cb_leave(channel);
@@ -271,9 +275,14 @@ ares_status_t ares_send_dnsrec(ares_channel_t *channel,
}
ares_channel_lock(channel);
+ /* Notifications raised along the way are delivered once this whole call
+ * has unwound, not from inside the internals it runs. See
+ * ares_deferred_cb_enter(). */
+ ares_deferred_cb_enter(channel);
status = ares_send_nolock(channel, NULL, 0, dnsrec, callback, arg, qid);
+ ares_deferred_cb_leave(channel);
ares_channel_unlock(channel);
return status;
diff --git a/test/ares-test-mock.cc b/test/ares-test-mock.cc
index 2158c5a913..b848c02434 100644
--- a/test/ares-test-mock.cc
+++ b/test/ares-test-mock.cc
@@ -2533,6 +2533,112 @@ TEST_F(LibraryTest, ReconfigureInSockStateCallbackNoUseAfterFree) {
data.channel = nullptr; /* don't reconfigure from teardown notifications */
ares_destroy(chan);
}
+// Server-state notifications raised while a public call is issuing its first
+// request must not be delivered until that whole call has unwound. A
+// failover handler that reconfigures the servers ends the in-flight request,
+// and with it the state the issuing call still has to finish with. The IPv6
+// server cannot get a socket, so the first send fails over to the IPv4 one
+// and queues a failure notification for the handler below.
+static ares_socket_t FailoverNoIPv6Socket(int af, int type, int protocol,
+ void *user_data)
+{
+ (void)user_data;
+ if (af == AF_INET6) {
+ return ARES_SOCKET_BAD;
+ }
+ return socket(af, type, protocol);
+}
+
+static void FailoverOnServerDown(const char *server_string, ares_bool_t success,
+ int flags, void *data)
+{
+ (void)server_string;
+ (void)flags;
+ if (!success) {
+ ares_set_servers_csv(static_cast<ares_channel_t *>(data),
+ "127.0.0.1:53063");
+ }
+}
+
+static ares_channel_t *FailoverChannel(ares_socket_functions *funcs)
+{
+ ares_channel_t *chan = nullptr;
+ struct ares_options opts;
+
+ memset(&opts, 0, sizeof(opts));
+ opts.tries = 1;
+ opts.flags = ARES_FLAG_NOSEARCH;
+ EXPECT_EQ(ARES_SUCCESS,
+ ares_init_options(&chan, &opts, ARES_OPT_TRIES | ARES_OPT_FLAGS));
+ if (chan == nullptr) {
+ return nullptr;
+ }
+ EXPECT_EQ(ARES_SUCCESS,
+ ares_set_servers_csv(chan, "[::1]:53061,127.0.0.1:53062"));
+ ares_set_server_state_callback(chan, FailoverOnServerDown, chan);
+
+ memset(funcs, 0, sizeof(*funcs));
+ funcs->asocket = FailoverNoIPv6Socket;
+ ares_set_socket_functions(chan, funcs, NULL);
+ return chan;
+}
+
+TEST_F(LibraryTest, ServerFailoverDuringGetAddrInfo) {
+ ares_socket_functions funcs;
+ struct ares_addrinfo_hints hints;
+ AddrInfoResult result;
+ ares_channel_t *chan = FailoverChannel(&funcs);
+ ASSERT_NE(nullptr, chan);
+
+ memset(&hints, 0, sizeof(hints));
+ hints.ai_family = AF_INET;
+ ares_getaddrinfo(chan, "www.google.com", NULL, &hints, AddrInfoCallback,
+ &result);
+
+ ares_cancel(chan);
+ EXPECT_TRUE(result.done_);
+ ares_destroy(chan);
+}
+
+// Same, through ares_query_dnsrec(): the query id must already have been
+// reported to the caller by the time the query can complete.
+struct FailoverQueryData {
+ unsigned short qid;
+ unsigned short qid_at_callback;
+ bool done;
+};
+
+static void FailoverQueryCallback(void *data, ares_status_t status,
+ size_t timeouts,
+ const ares_dns_record_t *dnsrec)
+{
+ FailoverQueryData *d = static_cast<FailoverQueryData *>(data);
+ (void)status;
+ (void)timeouts;
+ (void)dnsrec;
+ d->qid_at_callback = d->qid;
+ d->done = true;
+}
+
+TEST_F(LibraryTest, ServerFailoverDuringQuery) {
+ ares_socket_functions funcs;
+ FailoverQueryData data;
+ ares_channel_t *chan = FailoverChannel(&funcs);
+ ASSERT_NE(nullptr, chan);
+
+ data.qid = 0;
+ data.qid_at_callback = 0;
+ data.done = false;
+ EXPECT_EQ(ARES_SUCCESS,
+ ares_query_dnsrec(chan, "www.google.com", ARES_CLASS_IN,
+ ARES_REC_TYPE_A, FailoverQueryCallback, &data,
+ &data.qid));
+
+ EXPECT_TRUE(data.done);
+ EXPECT_EQ(data.qid, data.qid_at_callback);
+ ares_destroy(chan);
+}
+
// Deferring the notifications must not lose them: every public entry point
// that closes a socket still has to tell the application about it before it
@@ -0,0 +1,575 @@
From c8bd9ab53329f66ec0c395a11002a5696669b147 Mon Sep 17 00:00:00 2001
From: Brad House <brad@brad-house.com>
Date: Mon, 19 Oct 2026 11:53:54 -0400
Subject: [PATCH] [Backport v1.34] ares_process: only accept an answer on the
connection its query was sent on (#1361)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Backport of b6f8a9b47e17c681e092a74d05d73bbb35c89a39 to `v1.34`. See
that commit for the full description.
**Conflicts:** `src/lib/ares_socket.h` only — v1.34 aligns the
neighbouring prototypes differently. Resolved by taking the new doc
comments and the `ares_sockaddr_addr_port_eq()` prototype, then
re-running clang-format for the v1.34 alignment (which also re-wraps the
new `SendData()` declaration in `test/ares-test.h`). All other files
applied unchanged; the `test/` diffs match main exactly.
**Tests (local, macOS, Debug, Ninja):**
- ASAN+UBSAN and `-DCARES_WERROR=ON` builds: `arestest -4
--gtest_filter='-*Live*'` — 693/693 passed in both.
- With the `src/` changes reverted, the 4 new test instances fail
(`UDPAnswerFromWrongPortDropped`, `AnswerOnOtherServerConnectionDropped`
UDP/TCP, `CookielessAnswerOnOtherServerDropped`); they pass with them.
- `git clang-format --diff upstream/v1.34` clean. Also tested together
with the other two v1.34 backports (both orders apply cleanly): 699/699
passed under ASAN+UBSAN and `-Werror`.
Signed-off-by: Brad House <brad@brad-house.com>
Upstream-Status: Backport [https://github.com/c-ares/c-ares/commit/c8bd9ab53329f66ec0c395a11002a5696669b147]
GHSA: GHSA-fqh5-3x7m-wp44
---
src/lib/ares_conn.c | 11 ++-
src/lib/ares_process.c | 18 ++++
src/lib/ares_socket.c | 24 +++++
src/lib/ares_socket.h | 21 ++++
test/ares-test-mock.cc | 218 +++++++++++++++++++++++++++++++++++++++++
test/ares-test.cc | 86 +++++++++-------
test/ares-test.h | 12 +++
7 files changed, 355 insertions(+), 35 deletions(-)
diff --git a/src/lib/ares_conn.c b/src/lib/ares_conn.c
index 5e0827fbdf..b868053e62 100644
--- a/src/lib/ares_conn.c
+++ b/src/lib/ares_conn.c
@@ -252,9 +252,16 @@ ares_conn_err_t ares_conn_read(ares_conn_t *conn, void *data, size_t len,
(struct sockaddr *)&sa_storage, &salen, read_bytes);
#ifdef HAVE_RECVFROM
+ /* The socket is connect()ed, so the kernel normally drops datagrams from
+ * any other address or port already. Application-provided socket
+ * functions might not filter on connect, so also check here that the
+ * datagram came from the server's address and UDP port. RFC 5452
+ * section 9.1 only requires the address to match; matching the port too
+ * keeps one server from answering for another on the same address. */
if (err == ARES_CONN_ERR_SUCCESS &&
- !ares_sockaddr_addr_eq((struct sockaddr *)&sa_storage,
- &conn->server->addr)) {
+ !ares_sockaddr_addr_port_eq((struct sockaddr *)&sa_storage,
+ &conn->server->addr,
+ conn->server->udp_port)) {
err = ARES_CONN_ERR_WOULDBLOCK;
}
#endif
diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c
index 5686b077a6..06d0ca507c 100644
--- a/src/lib/ares_process.c
+++ b/src/lib/ares_process.c
@@ -1117,6 +1117,24 @@ static ares_status_t process_answer(ares_channel_t *channel,
goto cleanup;
}
+ /* RFC 5452 section 9.1: only accept a response that arrives on the
+ * connection the query was sent on, and so from the server it was sent to.
+ * For UDP, ares_conn_read() also drops a datagram whose source address or
+ * port is not the server's; matching the port is hardening beyond RFC 5452
+ * section 9.1, which only requires the address to match. Otherwise any
+ * server we hold a connection to could answer, and get cached, a query that
+ * was only ever sent to some other server, and the cookie check below would
+ * run against the wrong server's cookie state. Treat it like an unknown
+ * query id, before anything acts on it (cookies, server health, the cache).
+ * Our caller has already consumed the message from the connection's buffer,
+ * so TCP framing stays in sync. A query not attached to any connection
+ * (query->conn == NULL) never matches. This also drops a late answer from
+ * a server tried before a retry. */
+ if (query->conn != conn) {
+ status = ARES_SUCCESS;
+ goto cleanup;
+ }
+
/* Validate DNS cookie in response. This function may need to requeue the
* query. */
if (ares_cookie_validate(query, rdnsrec, conn, now, requeue)
diff --git a/src/lib/ares_socket.c b/src/lib/ares_socket.c
index 4ad3034021..0a12935ab7 100644
--- a/src/lib/ares_socket.c
+++ b/src/lib/ares_socket.c
@@ -127,6 +127,30 @@ ares_bool_t ares_sockaddr_addr_eq(const struct sockaddr *sa,
return ARES_FALSE; /* different */
}
+ares_bool_t ares_sockaddr_addr_port_eq(const struct sockaddr *sa,
+ const struct ares_addr *aa,
+ unsigned short port)
+{
+ unsigned short sa_port;
+
+ if (!ares_sockaddr_addr_eq(sa, aa)) {
+ return ARES_FALSE;
+ }
+
+ /* ares_sockaddr_addr_eq() only matches AF_INET and AF_INET6 */
+ if (sa->sa_family == AF_INET) {
+ sa_port = (CARES_INADDR_CAST(const struct sockaddr_in *, sa))->sin_port;
+ } else {
+ sa_port = (CARES_INADDR_CAST(const struct sockaddr_in6 *, sa))->sin6_port;
+ }
+
+ if (ntohs(sa_port) != port) {
+ return ARES_FALSE;
+ }
+
+ return ARES_TRUE;
+}
+
ares_conn_err_t ares_socket_write(ares_channel_t *channel, ares_socket_t fd,
const void *data, size_t len, size_t *written,
const struct sockaddr *sa,
diff --git a/src/lib/ares_socket.h b/src/lib/ares_socket.h
index 24a99ab331..f7128e3493 100644
--- a/src/lib/ares_socket.h
+++ b/src/lib/ares_socket.h
@@ -139,8 +139,29 @@ typedef enum {
ARES_CONN_ERR_FAILURE = 99 /*!< Generic failure */
} ares_conn_err_t;
+/*! Compare the IP address of a socket address with an ares_addr. The port
+ * in the socket address is ignored.
+ *
+ * \param[in] sa Socket address (AF_INET or AF_INET6)
+ * \param[in] aa Address to compare against
+ * \return ARES_TRUE if the address families and IP addresses are equal,
+ * ARES_FALSE otherwise
+ */
ares_bool_t ares_sockaddr_addr_eq(const struct sockaddr *sa,
const struct ares_addr *aa);
+
+/*! Compare the IP address and port of a socket address with an ares_addr and
+ * port.
+ *
+ * \param[in] sa Socket address (AF_INET or AF_INET6)
+ * \param[in] aa Address to compare against
+ * \param[in] port Port to compare against, in host byte order
+ * \return ARES_TRUE if the address families, IP addresses and ports are
+ * equal, ARES_FALSE otherwise
+ */
+ares_bool_t ares_sockaddr_addr_port_eq(const struct sockaddr *sa,
+ const struct ares_addr *aa,
+ unsigned short port);
ares_status_t ares_socket_configure(ares_channel_t *channel, int family,
ares_bool_t is_tcp, ares_socket_t fd);
ares_conn_err_t ares_socket_enable_tfo(const ares_channel_t *channel,
diff --git a/test/ares-test-mock.cc b/test/ares-test-mock.cc
index d233f9070f..058dc93001 100644
--- a/test/ares-test-mock.cc
+++ b/test/ares-test-mock.cc
@@ -3363,6 +3363,7 @@ struct FakeSockData {
unsigned char query[512];
size_t query_len;
struct sockaddr_in peer;
+ unsigned short recv_port; /* network order; 0 reports the peer's port */
size_t recv_calls;
size_t bad_fd_calls;
size_t sock_state_bad;
@@ -3471,6 +3472,9 @@ static ares_ssize_t FakeSockRecvfrom(ares_socket_t sock, void *buffer,
if (address != NULL && address_len != NULL &&
*address_len >= (ares_socklen_t)sizeof(d->peer)) {
memcpy(address, &d->peer, sizeof(d->peer));
+ if (d->recv_port != 0) {
+ reinterpret_cast<struct sockaddr_in *>(address)->sin_port = d->recv_port;
+ }
*address_len = sizeof(d->peer);
}
return static_cast<ares_ssize_t>(d->query_len);
@@ -3710,6 +3714,42 @@ TEST_F(LibraryTest, ReconfigureInRecvDropsBufferedAnswer) {
ares_destroy(chan);
}
+// Socket functions need not filter on connect(), so ares_conn_read() checks
+// a UDP datagram's source itself. An answer reported from the server's
+// address but another port (here 127.0.0.1:53 for a server at
+// 127.0.0.1:53055) must be dropped, so that one server cannot answer for
+// another on the same address. The same answer from the right port is then
+// accepted.
+TEST_F(LibraryTest, UDPAnswerFromWrongPortDropped) {
+ ares_channel_t *chan = nullptr;
+ FakeSockData data;
+
+ memset(&data, 0, sizeof(data));
+ FakeSockInit(&chan, &data, 0, ARES_SOCKFUNC_FLAG_NONBLOCKING);
+
+ ares_query_dnsrec(chan, "www.google.com", ARES_CLASS_IN, ARES_REC_TYPE_A,
+ FakeSockQueryCallback, &data, NULL);
+ ASSERT_NE((size_t)0, data.query_len);
+ ASSERT_EQ(htons(53055), data.peer.sin_port);
+
+ data.recv_port = htons(53);
+ data.recv_answer = true;
+ ares_process_fd(chan, 100, ARES_SOCKET_BAD);
+ EXPECT_FALSE(data.recv_answer);
+ EXPECT_FALSE(data.done);
+ EXPECT_EQ((size_t)1, ares_queue_active_queries(chan));
+
+ data.recv_port = 0;
+ data.recv_answer = true;
+ ares_process_fd(chan, 100, ARES_SOCKET_BAD);
+ EXPECT_FALSE(data.recv_answer);
+ EXPECT_TRUE(data.done);
+ // The fake answer carries no records.
+ EXPECT_EQ(ARES_ENODATA, data.status);
+ EXPECT_EQ((size_t)0, ares_queue_active_queries(chan));
+ ares_destroy(chan);
+}
+
// The close function is application code too. With no socket-state callback
// to carry the descriptor, it runs inline while ares_check_cleanup_conns()
// walks the server list and each server's connections, closing idle ones.
@@ -5530,5 +5570,183 @@ INSTANTIATE_TEST_SUITE_P(TransportModes, ServerRecoveryMultiMockTest, ::testing:
INSTANTIATE_TEST_SUITE_P(TransportModes, ServerRecoveryNoProbeMultiMockTest, ::testing::ValuesIn(ares::test::families_modes), PrintFamilyMode);
+// An answer is only accepted on the connection its query was sent on
+// (RFC 5452 section 9.1). Two servers: #0 is demoted by a SERVFAIL so a later
+// query goes only to #1, while #0's connection stays open (STAYOPEN). Server
+// #0 then answers that query on its own connection. The answer must be
+// dropped without side effects, and the query must then complete normally.
+class ReplyConnMultiMockTest
+ : public MockChannelOptsTest,
+ public ::testing::WithParamInterface< std::pair<int, bool> > {
+ public:
+ ReplyConnMultiMockTest()
+ : MockChannelOptsTest(2, GetParam().first, GetParam().second, false,
+ FillOptions(&opts_),
+ ARES_OPT_FLAGS | ARES_OPT_TRIES |
+ ARES_OPT_SERVER_FAILOVER | ARES_OPT_NOROTATE) {}
+
+ static struct ares_options* FillOptions(struct ares_options *opts) {
+ memset(opts, 0, sizeof(struct ares_options));
+ /* Keep the demoted server's connection open; no DNS 0x20 so the replies
+ * built here match the question exactly */
+ opts->flags = ARES_FLAG_STAYOPEN | ARES_FLAG_EDNS;
+ /* One try per server: the query goes to #1, then on timeout to #0 */
+ opts->tries = 1;
+ /* Never probe the demoted server, so the query is sent only to #1 */
+ opts->server_failover_opts.retry_chance = 0;
+ opts->server_failover_opts.retry_delay = 10000;
+ return opts;
+ }
+ private:
+ struct ares_options opts_;
+};
+
+static std::string ReplyConnFirstA(const QueryResult &result)
+{
+ const ares_dns_rr_t *rr;
+ char addr[64];
+
+ if (result.dnsrec_.dnsrec_ == nullptr ||
+ ares_dns_record_rr_cnt(result.dnsrec_.dnsrec_, ARES_SECTION_ANSWER) == 0) {
+ return "";
+ }
+ rr = ares_dns_record_rr_get_const(result.dnsrec_.dnsrec_,
+ ARES_SECTION_ANSWER, 0);
+ if (ares_dns_rr_get_type(rr) != ARES_REC_TYPE_A ||
+ ares_inet_ntop(AF_INET, ares_dns_rr_get_addr(rr, ARES_RR_A_ADDR), addr,
+ sizeof(addr)) == nullptr) {
+ return "";
+ }
+ return addr;
+}
+
+TEST_P(ReplyConnMultiMockTest, AnswerOnOtherServerConnectionDropped) {
+ DNSPacket warm_servfail;
+ warm_servfail.set_response().set_aa().set_rcode(SERVFAIL)
+ .add_question(new DNSQuestion("warmup.example", T_A));
+ DNSPacket warm_ok;
+ warm_ok.set_response().set_aa()
+ .add_question(new DNSQuestion("warmup.example", T_A))
+ .add_answer(new DNSARR("warmup.example", 100, {1, 1, 1, 1}));
+ DNSPacket forged;
+ forged.set_response().set_aa()
+ .add_question(new DNSQuestion("victim.example", T_A))
+ .add_answer(new DNSARR("victim.example", 100, {6, 6, 6, 6}));
+ DNSPacket genuine;
+ genuine.set_response().set_aa()
+ .add_question(new DNSQuestion("victim.example", T_A))
+ .add_answer(new DNSARR("victim.example", 100, {1, 2, 3, 4}));
+ unsigned short qid = 0;
+
+ /* Demote #0, leaving a connection to it open */
+ EXPECT_CALL(*servers_[0], OnRequest("warmup.example", T_A))
+ .WillOnce(SetReply(servers_[0].get(), &warm_servfail));
+ EXPECT_CALL(*servers_[1], OnRequest("warmup.example", T_A))
+ .WillOnce(SetReply(servers_[1].get(), &warm_ok));
+ QueryResult warm;
+ ares_query_dnsrec(channel_, "warmup.example", ARES_CLASS_IN,
+ ARES_REC_TYPE_A, QueryCallback, &warm, NULL);
+ Process();
+ ASSERT_TRUE(warm.done_);
+ ASSERT_EQ(ARES_SUCCESS, warm.status_);
+
+ /* The query goes only to #1, which does not answer. Meanwhile #0 answers
+ * it on #0's own connection. That answer must be dropped, so the query
+ * times out on #1 and is retried on #0, whose genuine answer is accepted on
+ * the same (still in sync, for TCP) connection. */
+ EXPECT_CALL(*servers_[1], OnRequest("victim.example", T_A))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ servers_[0]->SendUnsolicited(&forged, qid);
+ servers_[1]->SetReplyData(std::vector<byte>());
+ }));
+ EXPECT_CALL(*servers_[0], OnRequest("victim.example", T_A))
+ .WillOnce(SetReply(servers_[0].get(), &genuine));
+ QueryResult result;
+ ares_query_dnsrec(channel_, "victim.example", ARES_CLASS_IN,
+ ARES_REC_TYPE_A, QueryCallback, &result, &qid);
+ Process();
+ ASSERT_TRUE(result.done_);
+ EXPECT_EQ(ARES_SUCCESS, result.status_);
+ EXPECT_EQ("1.2.3.4", ReplyConnFirstA(result));
+ EXPECT_EQ(1, (int)result.timeouts_);
+}
+
+// DNS cookies (RFC 7873) are per-server state, validated against the server
+// the query was sent to. #1 supports cookies, #0 does not. A cookie-less
+// answer arriving on #0's connection for a query sent only to #1 must not be
+// accepted: it would otherwise be checked against #0's cookie state and pass.
+class ReplyConnCookieMultiMockTest
+ : public MockChannelOptsTest,
+ public ::testing::WithParamInterface<int> {
+ public:
+ ReplyConnCookieMultiMockTest()
+ : MockChannelOptsTest(2, GetParam(), false, false,
+ ReplyConnMultiMockTest::FillOptions(&opts_),
+ ARES_OPT_FLAGS | ARES_OPT_TRIES |
+ ARES_OPT_SERVER_FAILOVER | ARES_OPT_NOROTATE) {}
+ private:
+ struct ares_options opts_;
+};
+
+TEST_P(ReplyConnCookieMultiMockTest, CookielessAnswerOnOtherServerDropped) {
+ std::vector<byte> server_cookie = { 1, 2, 3, 4, 5, 6, 7, 8 };
+ DNSPacket warm_servfail;
+ warm_servfail.set_response().set_aa().set_rcode(SERVFAIL)
+ .add_question(new DNSQuestion("warmup.example", T_A));
+ DNSPacket warm_ok;
+ warm_ok.set_response().set_aa()
+ .add_question(new DNSQuestion("warmup.example", T_A))
+ .add_answer(new DNSARR("warmup.example", 100, {1, 1, 1, 1}))
+ .add_additional(new DNSOptRR(0, 0, 0, 1280, { }, server_cookie, false));
+ DNSPacket forged;
+ forged.set_response().set_aa()
+ .add_question(new DNSQuestion("victim.example", T_A))
+ .add_answer(new DNSARR("victim.example", 100, {6, 6, 6, 6}));
+ DNSPacket genuine;
+ genuine.set_response().set_aa()
+ .add_question(new DNSQuestion("victim.example", T_A))
+ .add_answer(new DNSARR("victim.example", 100, {1, 2, 3, 4}));
+ unsigned short qid = 0;
+
+ /* #0 answers without a cookie (and is demoted); #1 answers with one, so
+ * #1's cookie state becomes SUPPORTED */
+ EXPECT_CALL(*servers_[0], OnRequest("warmup.example", T_A))
+ .WillOnce(SetReply(servers_[0].get(), &warm_servfail));
+ EXPECT_CALL(*servers_[1], OnRequest("warmup.example", T_A))
+ .WillOnce(SetReply(servers_[1].get(), &warm_ok));
+ QueryResult warm;
+ ares_query_dnsrec(channel_, "warmup.example", ARES_CLASS_IN,
+ ARES_REC_TYPE_A, QueryCallback, &warm, NULL);
+ Process();
+ ASSERT_TRUE(warm.done_);
+ ASSERT_EQ(ARES_SUCCESS, warm.status_);
+ size_t cookie_len = 0;
+ fetch_server_cookie(warm.dnsrec_.dnsrec_, &cookie_len);
+ ASSERT_EQ(server_cookie.size(), cookie_len);
+
+ /* The cookie-protected query goes only to #1; a cookie-less answer for it
+ * arrives on #0's connection and must be dropped. The retry to #0 is then
+ * legitimately answered by #0, which does not use cookies. */
+ EXPECT_CALL(*servers_[1], OnRequest("victim.example", T_A))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ servers_[0]->SendUnsolicited(&forged, qid);
+ servers_[1]->SetReplyData(std::vector<byte>());
+ }));
+ EXPECT_CALL(*servers_[0], OnRequest("victim.example", T_A))
+ .WillOnce(SetReply(servers_[0].get(), &genuine));
+ QueryResult result;
+ ares_query_dnsrec(channel_, "victim.example", ARES_CLASS_IN,
+ ARES_REC_TYPE_A, QueryCallback, &result, &qid);
+ Process();
+ ASSERT_TRUE(result.done_);
+ EXPECT_EQ(ARES_SUCCESS, result.status_);
+ EXPECT_EQ("1.2.3.4", ReplyConnFirstA(result));
+ EXPECT_EQ(1, (int)result.timeouts_);
+}
+
+INSTANTIATE_TEST_SUITE_P(TransportModes, ReplyConnMultiMockTest, ::testing::ValuesIn(ares::test::families_modes), PrintFamilyMode);
+
+INSTANTIATE_TEST_SUITE_P(AddressFamilies, ReplyConnCookieMultiMockTest, ::testing::ValuesIn(ares::test::families), PrintFamily);
+
} // namespace test
} // namespace ares
diff --git a/test/ares-test.cc b/test/ares-test.cc
index d333ee9419..0b3ac12c09 100644
--- a/test/ares-test.cc
+++ b/test/ares-test.cc
@@ -448,6 +448,9 @@ MockServer::MockServer(int family, unsigned short port)
// Create a TCP socket to receive data on.
tcp_data_ = NULL;
tcp_data_len_ = 0;
+ last_fd_ = ARES_SOCKET_BAD;
+ memset(&last_addr_, 0, sizeof(last_addr_));
+ last_addrlen_ = 0;
tcpfd_ = socket(family, SOCK_STREAM, 0);
EXPECT_NE(ARES_SOCKET_BAD, tcpfd_);
int optval = 1;
@@ -710,6 +713,50 @@ std::set<ares_socket_t> MockServer::fds() const {
return result;
}
+void MockServer::SendData(ares_socket_t fd, struct sockaddr_storage *addr,
+ ares_socklen_t addrlen, int qid,
+ std::vector<byte> reply) {
+ int flags = 0;
+
+ if (reply.size() >= 2) {
+ // Overwrite the query ID if space to do so.
+ reply[0] = (byte)((qid >> 8) & 0xff);
+ reply[1] = (byte)(qid & 0xff);
+ }
+ if (verbose) {
+ std::cerr << "sending reply " << PacketToString(reply)
+ << " on port " << ((fd == udpfd_) ? udpport_ : tcpport_)
+ << ":" << getaddrport(addr) << std::endl;
+ }
+
+ // Prefix with 2-byte length if TCP.
+ if (fd != udpfd_) {
+ int len = (int)reply.size();
+ std::vector<byte> vlen = {(byte)((len & 0xFF00) >> 8), (byte)(len & 0xFF)};
+ reply.insert(reply.begin(), vlen.begin(), vlen.end());
+ // Also, don't bother with the destination address.
+ addr = nullptr;
+ addrlen = 0;
+ }
+
+#ifdef MSG_NOSIGNAL
+ flags |= MSG_NOSIGNAL;
+#endif
+
+ ares_ssize_t rc = (ares_ssize_t)sendto(fd, BYTE_CAST reply.data(), (SEND_TYPE_ARG3)reply.size(), flags,
+ (struct sockaddr *)addr, addrlen);
+ if (rc < static_cast<ares_ssize_t>(reply.size())) {
+ std::cerr << "Failed to send full reply, rc=" << rc << std::endl;
+ }
+}
+
+void MockServer::SendUnsolicited(const DNSPacket *reply, unsigned short qid) {
+ ASSERT_NE(ARES_SOCKET_BAD, last_fd_);
+ ASSERT_TRUE(last_fd_ == udpfd_ || connfds_.count(last_fd_) != 0)
+ << "the most recent requester's connection has closed";
+ SendData(last_fd_, &last_addr_, last_addrlen_, qid, reply->data());
+}
+
void MockServer::ProcessRequest(ares_socket_t fd, struct sockaddr_storage* addr,
ares_socklen_t addrlen, const std::vector<byte> &req,
const std::string &reqstr,
@@ -717,10 +764,14 @@ void MockServer::ProcessRequest(ares_socket_t fd, struct sockaddr_storage* addr,
/* DNS 0x20 will mix case, do case-insensitive matching of name in request */
char lower_name[256];
- int flags = 0;
arestest_strtolower(lower_name, name, sizeof(lower_name));
+ // Remember the requester so SendUnsolicited() can reach it.
+ last_fd_ = fd;
+ memcpy(&last_addr_, addr, sizeof(last_addr_));
+ last_addrlen_ = addrlen;
+
// Before processing, let gMock know the request is happening.
OnRequest(lower_name, rrtype);
@@ -747,43 +798,12 @@ void MockServer::ProcessRequest(ares_socket_t fd, struct sockaddr_storage* addr,
return;
}
- // Make a local copy of the current pending reply.
- std::vector<byte> reply = exact_reply_;
-
if (qid_ >= 0) {
// Use the explicitly specified query ID.
qid = qid_;
}
- if (reply.size() >= 2) {
- // Overwrite the query ID if space to do so.
- reply[0] = (byte)((qid >> 8) & 0xff);
- reply[1] = (byte)(qid & 0xff);
- }
- if (verbose) {
- std::cerr << "sending reply " << PacketToString(reply)
- << " on port " << ((fd == udpfd_) ? udpport_ : tcpport_)
- << ":" << getaddrport(addr) << std::endl;
- }
-
- // Prefix with 2-byte length if TCP.
- if (fd != udpfd_) {
- int len = (int)reply.size();
- std::vector<byte> vlen = {(byte)((len & 0xFF00) >> 8), (byte)(len & 0xFF)};
- reply.insert(reply.begin(), vlen.begin(), vlen.end());
- // Also, don't bother with the destination address.
- addr = nullptr;
- addrlen = 0;
- }
-#ifdef MSG_NOSIGNAL
- flags |= MSG_NOSIGNAL;
-#endif
-
- ares_ssize_t rc = (ares_ssize_t)sendto(fd, BYTE_CAST reply.data(), (SEND_TYPE_ARG3)reply.size(), flags,
- (struct sockaddr *)addr, addrlen);
- if (rc < static_cast<ares_ssize_t>(reply.size())) {
- std::cerr << "Failed to send full reply, rc=" << rc << std::endl;
- }
+ SendData(fd, addr, addrlen, qid, exact_reply_);
if (disconnect_after_reply_ && fd != udpfd_) {
disconnect_after_reply_ = false;
diff --git a/test/ares-test.h b/test/ares-test.h
index 2b973c3982..170c284f26 100644
--- a/test/ares-test.h
+++ b/test/ares-test.h
@@ -303,6 +303,12 @@ class MockServer {
disconnect_after_reply_ = true;
}
+ // Send reply, with the given query ID, to the client of the most recent
+ // request this server received, on the socket that request arrived on. The
+ // client did not send this server a request with that query ID: this models
+ // a server answering a query that was only sent to some other server.
+ void SendUnsolicited(const DNSPacket *reply, unsigned short qid);
+
// The set of file descriptors that the server handles.
std::set<ares_socket_t> fds() const;
@@ -327,6 +333,8 @@ class MockServer {
int rrtype);
void ProcessPacket(ares_socket_t fd, struct sockaddr_storage *addr,
ares_socklen_t addrlen, byte *data, int len);
+ void SendData(ares_socket_t fd, struct sockaddr_storage *addr,
+ ares_socklen_t addrlen, int qid, std::vector<byte> reply);
unsigned short udpport_;
unsigned short tcpport_;
ares_socket_t udpfd_;
@@ -339,6 +347,10 @@ class MockServer {
bool disconnect_after_reply_;
unsigned char *tcp_data_;
size_t tcp_data_len_;
+ // Where the most recent request came from, for SendUnsolicited().
+ ares_socket_t last_fd_;
+ struct sockaddr_storage last_addr_;
+ ares_socklen_t last_addrlen_;
};
// Test fixture that uses a mock DNS server.
@@ -0,0 +1,243 @@
From 25a6911370878d4b617b44b142ea0e49d1ccede6 Mon Sep 17 00:00:00 2001
From: Brad House <brad@brad-house.com>
Date: Mon, 19 Oct 2026 11:53:59 -0400
Subject: [PATCH] [Backport v1.34] ares_save_options: read channel
configuration under the channel lock (#1362)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Backport of 5d878a580a8b57b51d15e2725cf6775c8e7bca0e to `v1.34`. See
that commit for the full description.
**Conflicts:** none; cherry-picked cleanly.
**Tests (local, macOS, Debug, Ninja):**
- ASAN+UBSAN and `-DCARES_WERROR=ON` builds: `arestest -4
--gtest_filter='-*Live*'` — 691/691 passed in both.
- With the `src/` changes reverted, the new `DupConcurrentSetServers` /
`SaveOptionsConcurrentSetServers` tests abort with an ASAN
heap-buffer-overflow; they pass with them.
- `git clang-format --diff upstream/v1.34` clean. Also tested together
with the other two v1.34 backports (both orders apply cleanly): 699/699
passed under ASAN+UBSAN and `-Werror`.
Signed-off-by: Brad House <brad@brad-house.com>
Upstream-Status: Backport [https://github.com/c-ares/c-ares/commit/25a6911370878d4b617b44b142ea0e49d1ccede6]
GHSA: GHSA-ph6c-3cvx-mq6v
---
src/lib/ares_init.c | 3 +-
src/lib/ares_options.c | 45 ++++++++++++++++++----
test/ares-test-init.cc | 87 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 126 insertions(+), 9 deletions(-)
diff --git a/src/lib/ares_init.c b/src/lib/ares_init.c
index 4758c5de56..85b2e13050 100644
--- a/src/lib/ares_init.c
+++ b/src/lib/ares_init.c
@@ -483,7 +483,8 @@ int ares_dup(ares_channel_t **dest, const ares_channel_t *src)
*dest = NULL; /* in case of failure return NULL explicitly */
/* First get the options supported by the old ares_save_options() function,
- which is most of them */
+ * which is most of them. ares_save_options() takes the src channel lock
+ * itself. No lock is held across ares_init_options(). */
rc = (ares_status_t)ares_save_options(src, &opts, &optmask);
if (rc != ARES_SUCCESS) {
ares_destroy_options(&opts);
diff --git a/src/lib/ares_options.c b/src/lib/ares_options.c
index 2b5624bbea..5ea35b732e 100644
--- a/src/lib/ares_options.c
+++ b/src/lib/ares_options.c
@@ -55,8 +55,8 @@ static struct in_addr *ares_save_opt_servers(const ares_channel_t *channel,
int *nservers)
{
ares_slist_node_t *snode;
- struct in_addr *out =
- ares_malloc_zero(ares_slist_len(channel->servers) * sizeof(*out));
+ size_t cnt = ares_slist_len(channel->servers);
+ struct in_addr *out = ares_malloc_zero_array(cnt, sizeof(*out));
*nservers = 0;
@@ -64,7 +64,11 @@ static struct in_addr *ares_save_opt_servers(const ares_channel_t *channel,
return NULL;
}
- for (snode = ares_slist_node_first(channel->servers); snode != NULL;
+ /* Never write more entries than were allocated. The caller holds the
+ * channel lock so the server list cannot change underneath us; this is
+ * purely defensive. */
+ for (snode = ares_slist_node_first(channel->servers);
+ snode != NULL && (size_t)*nservers < cnt;
snode = ares_slist_node_next(snode)) {
const ares_server_t *server = ares_slist_node_val(snode);
@@ -79,9 +83,11 @@ static struct in_addr *ares_save_opt_servers(const ares_channel_t *channel,
return out;
}
-/* Save options from initialized channel */
-int ares_save_options(const ares_channel_t *channel,
- struct ares_options *options, int *optmask)
+/* Save options from initialized channel. Caller must hold the channel lock
+ * (if channel is not NULL). */
+static ares_status_t ares_save_options_nolock(const ares_channel_t *channel,
+ struct ares_options *options,
+ int *optmask)
{
size_t i;
@@ -150,7 +156,8 @@ int ares_save_options(const ares_channel_t *channel,
if (channel->optmask & ARES_OPT_DOMAINS) {
options->domains = NULL;
if (channel->ndomains) {
- options->domains = ares_malloc(channel->ndomains * sizeof(char *));
+ options->domains =
+ ares_malloc_zero_array(channel->ndomains, sizeof(char *));
if (!options->domains) {
return ARES_ENOMEM;
}
@@ -176,7 +183,8 @@ int ares_save_options(const ares_channel_t *channel,
if (channel->optmask & ARES_OPT_SORTLIST) {
options->sortlist = NULL;
if (channel->nsort) {
- options->sortlist = ares_malloc(channel->nsort * sizeof(struct apattern));
+ options->sortlist =
+ ares_malloc_zero_array(channel->nsort, sizeof(*options->sortlist));
if (!options->sortlist) {
return ARES_ENOMEM;
}
@@ -238,6 +246,27 @@ int ares_save_options(const ares_channel_t *channel,
return ARES_SUCCESS;
}
+int ares_save_options(const ares_channel_t *channel,
+ struct ares_options *options, int *optmask)
+{
+ ares_status_t status;
+
+ if (channel == NULL) {
+ /* Preserve historic behavior: initialize the pointers so
+ * ares_destroy_options() is safe, then return ARES_ENODATA */
+ return (int)ares_save_options_nolock(NULL, options, optmask);
+ }
+
+ /* The configuration may be modified concurrently by other threads (e.g.
+ * ares_set_servers_csv()), so it must only be read with the channel lock
+ * held. */
+ ares_channel_lock(channel);
+ status = ares_save_options_nolock(channel, options, optmask);
+ ares_channel_unlock(channel);
+
+ return (int)status;
+}
+
static ares_status_t ares_init_options_servers(ares_channel_t *channel,
const struct in_addr *servers,
size_t nservers)
diff --git a/test/ares-test-init.cc b/test/ares-test-init.cc
index b44211a1e4..1c5d0f2171 100644
--- a/test/ares-test-init.cc
+++ b/test/ares-test-init.cc
@@ -25,6 +25,8 @@
*/
#include "ares-test.h"
+#include <atomic>
+
extern "C" {
#include "ares_private.h"
}
@@ -295,6 +297,91 @@ TEST_F(LibraryTest, FailChannelInit) {
ares_library_cleanup();
}
+// CARES_THREADS implies ares_threadsafety() is true
+#ifdef CARES_THREADS
+// Build a CSV list of n distinct IPv4 servers.
+static std::string ConfigRaceServerCsv(size_t n)
+{
+ std::string csv;
+ for (size_t i = 0; i < n; i++) {
+ if (i != 0) {
+ csv += ",";
+ }
+ csv += "127.0." + std::to_string((i / 250) + 1) + "." +
+ std::to_string((i % 250) + 1) + ":5353";
+ }
+ return csv;
+}
+
+// Readers snapshot the channel configuration (via ares_dup() or
+// ares_save_options()) while a writer thread keeps swapping the server list
+// between a short and a long one. The configuration must only be read under
+// the channel lock; otherwise the saved server array is sized from one list
+// and filled from another (heap-buffer-overflow under ASAN).
+static void ConfigRaceRun(bool use_dup, size_t reader_loops)
+{
+ const size_t nreaders = 4;
+ const std::string small_csv = ConfigRaceServerCsv(4);
+ const std::string large_csv = ConfigRaceServerCsv(400);
+ ares_channel_t *channel = nullptr;
+ std::atomic<size_t> readers_done(0);
+
+ EXPECT_EQ(ARES_SUCCESS, ares_init_options(&channel, nullptr, 0));
+ ASSERT_NE(nullptr, channel);
+ EXPECT_EQ(ARES_SUCCESS, ares_set_servers_csv(channel, small_csv.c_str()));
+
+ // Writer runs until every reader finished; readers are bounded so the
+ // whole test is bounded.
+ std::thread writer([&]() {
+ size_t i = 0;
+ while (readers_done.load() < nreaders) {
+ ares_set_servers_csv(channel,
+ (i++ & 1) ? large_csv.c_str() : small_csv.c_str());
+ }
+ });
+
+ std::vector<std::thread> readers;
+ for (size_t r = 0; r < nreaders; r++) {
+ readers.emplace_back([&]() {
+ for (size_t i = 0; i < reader_loops; i++) {
+ if (use_dup) {
+ ares_channel_t *dup = nullptr;
+ EXPECT_EQ(ARES_SUCCESS, ares_dup(&dup, channel));
+ if (dup != nullptr) {
+ ares_destroy(dup);
+ }
+ } else {
+ struct ares_options opts;
+ int optmask = 0;
+ memset(&opts, 0, sizeof(opts));
+ EXPECT_EQ(ARES_SUCCESS, ares_save_options(channel, &opts, &optmask));
+ EXPECT_NE(0, optmask & ARES_OPT_SERVERS);
+ EXPECT_TRUE(opts.nservers == 4 || opts.nservers == 400);
+ ares_destroy_options(&opts);
+ }
+ }
+ readers_done++;
+ });
+ }
+
+ for (auto &t : readers) {
+ t.join();
+ }
+ writer.join();
+ ares_destroy(channel);
+}
+
+TEST_F(LibraryTest, DupConcurrentSetServers)
+{
+ ConfigRaceRun(true, 20);
+}
+
+TEST_F(LibraryTest, SaveOptionsConcurrentSetServers)
+{
+ ConfigRaceRun(false, 20000);
+}
+#endif
+
#ifndef WIN32
TEST_F(LibraryTest, EnvInit) {
ares_channel_t *channel = nullptr;
@@ -0,0 +1,364 @@
From 7e65b381c0b915aa5671ddc713080c87e15fc42f Mon Sep 17 00:00:00 2001
From: Brad House <brad@brad-house.com>
Date: Mon, 19 Oct 2026 12:24:52 -0400
Subject: [PATCH] [Backport v1.34] ares_inet_pton: reject a prefix length,
always write the full address (#1363)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Backport of 7b4ef8e555b611be6af39d913aa280cd1a348102 to `v1.34`. See
that commit for the full description.
**Conflicts:** `test/ares-test-mock-ai.cc` only — the neighbouring
`FamilyV6IgnoresARecords` test (added on main by an unrelated change)
does not exist on v1.34. Resolved by adding only the new
`NumericHostRejectsPrefixLength` test at the same position. Other files
applied unchanged.
**Also includes** the comment spelling fix from #1364
(ead5557d1301328536c3920bd01759a1bff5ca9a): "unparseable" ->
"unparsable" in `src/lib/inet_net_pton.c` and
`test/ares-test-internal.cc`, so Codespell passes and the text matches
main.
**Tests (local, macOS, Debug, Ninja):**
- ASAN+UBSAN and `-DCARES_WERROR=ON` builds: `arestest -4
--gtest_filter='-*Live*'` — 693/693 passed in both.
- With the `src/` changes reverted, the 4 new test instances fail
(`InetNetPtoNPrefixZeroFills`, `InetPtoNRejectsPrefixLength`,
`NumericHostRejectsPrefixLength` UDP/TCP); they pass with them.
- `git clang-format --diff upstream/v1.34` clean. Also tested together
with the other two v1.34 backports (both orders apply cleanly): 699/699
passed under ASAN+UBSAN and `-Werror`.
Signed-off-by: Brad House <brad@brad-house.com>
Upstream-Status: Backport [https://github.com/c-ares/c-ares/commit/7e65b381c0b915aa5671ddc713080c87e15fc42f]
GHSA: GHSA-cfcv-8v2j-qj6r
---
docs/ares_inet_pton.3 | 18 ++++++++--
src/lib/ares_inet_net_pton.h | 28 ++++++++++++++++
src/lib/inet_net_pton.c | 65 ++++++++++++++++++++----------------
test/ares-test-internal.cc | 54 ++++++++++++++++++++++++++++++
test/ares-test-mock-ai.cc | 23 +++++++++++++
5 files changed, 157 insertions(+), 31 deletions(-)
diff --git a/docs/ares_inet_pton.3 b/docs/ares_inet_pton.3
index 34b2df063c..19c5fdfda3 100644
--- a/docs/ares_inet_pton.3
+++ b/docs/ares_inet_pton.3
@@ -12,8 +12,9 @@ ares_inet_pton \- convert an IPv4 or IPv6 address from text to binary form
int ares_inet_pton(int \fIaf\fP, const char *\fIsrc\fP, void *\fIdst\fP);
.fi
.SH DESCRIPTION
-This is a portable version with the identical functionality of the commonly
-available \fIinet_pton\fP.
+This is a portable version of the commonly available \fIinet_pton\fP. It
+behaves the same for AF_INET6, and for AF_INET it additionally accepts the
+legacy forms described below.
The ares_inet_pton() function converts the address in its standard text
presentation form into its numeric binary form. The \fBaf\fP argument shall
@@ -23,9 +24,20 @@ in. The \fBdst\fP argument points to a buffer into which the function stores
the numeric address; this shall be large enough to hold the numeric address
(32 bits for AF_INET, 128 bits for AF_INET6).
+The \fBsrc\fP argument must be an address, not a network: a "/bits" prefix
+length suffix (CIDR notation) is not accepted and the function returns 0 for
+such input. On success, the whole address (4 bytes for AF_INET, 16 bytes for
+AF_INET6) is always written to \fBdst\fP. Versions of c-ares before 1.34.9
+accepted a prefix length here.
+
+Unlike POSIX \fIinet_pton\fP, AF_INET also accepts shortened forms such as
+"10.1" and hexadecimal forms such as "0x0a000001". Octets that are not given
+are set to zero, so "10.1" is 10.1.0.0.
+
It returns 1 if the address was valid for the specified address family, or 0
if the address was not parseable in the specified address family, or -1 if
-some system error occurred (in which case errno will have been set).
+the address family is not supported (in which case errno will have been set
+to EAFNOSUPPORT).
.SH SEE ALSO
.BR ares_init (3),
diff --git a/src/lib/ares_inet_net_pton.h b/src/lib/ares_inet_net_pton.h
index e3ed83a3cb..e1a6f2d71e 100644
--- a/src/lib/ares_inet_net_pton.h
+++ b/src/lib/ares_inet_net_pton.h
@@ -26,6 +26,34 @@
#ifndef HEADER_CARES_INET_NET_PTON_H
#define HEADER_CARES_INET_NET_PTON_H
+/*! Parse a network (an address with an optional "/bits" prefix length) from
+ * its text form. This is not an address parser: for an address use
+ * ares_inet_pton(), which rejects a prefix length and always writes the
+ * whole address.
+ *
+ * How it differs from ares_inet_pton():
+ * - A "/bits" prefix length is accepted (0-32 for IPv4, 0-128 for IPv6).
+ * - The return value is the prefix length, not 1. Without "/bits", IPv6
+ * returns 128 and IPv4 infers it from the address class, widened to
+ * cover every octet given ("10" is 8, "130.155" is 16, "1.2.3.4" is 32,
+ * and a lone "224" is 4). 0 is a valid result ("::1/0").
+ * - What is written to dst differs by family. On success all size bytes
+ * are zeroed first. IPv4 then writes every octet given, including bits
+ * beyond the prefix ("1.2.3.4/8" writes 1.2.3.4). IPv6 writes only the
+ * (bits+7)/8 bytes covering the prefix ("2001:db8::1/64" writes
+ * 2001:db8::).
+ *
+ * \param[in] af AF_INET or AF_INET6.
+ * \param[in] src NUL-terminated network in text form.
+ * \param[out] dst Buffer to receive the network, in network byte order.
+ * \param[in] size Size of dst in bytes.
+ * \return Prefix length in bits on success. -1 on failure, with SOCKERRNO
+ * set to ENOENT if src is not a valid network, or EMSGSIZE if it
+ * does not fit in size bytes or (IPv6) src is too long to be a
+ * network. On Windows these are the WSA equivalents. An
+ * unsupported af returns -1 without setting SOCKERRNO. On failure
+ * dst may be partially written.
+ */
int ares_inet_net_pton(int af, const char *src, void *dst, size_t size);
#endif /* HEADER_CARES_INET_NET_PTON_H */
diff --git a/src/lib/inet_net_pton.c b/src/lib/inet_net_pton.c
index e1f76ef834..f60823023b 100644
--- a/src/lib/inet_net_pton.c
+++ b/src/lib/inet_net_pton.c
@@ -51,14 +51,15 @@ const struct ares_in6_addr ares_in6addr_any = { { { 0, 0, 0, 0, 0, 0, 0, 0, 0,
/*
* static int
- * inet_net_pton_ipv4(src, dst, size)
+ * inet_net_pton_ipv4(src, dst, size, allow_cidr)
* convert IPv4 network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
+ * accepts hex octets, hex strings, decimal octets, and, if
+ * "allow_cidr" is true, /CIDR. "size" is in bytes and describes "dst".
* return:
* number of bits, either imputed classfully or specified with /CIDR,
* or -1 if some failure occurred (check errno). ENOENT means it was
- * not an IPv4 network specification.
+ * not an IPv4 network specification (including a /CIDR suffix when
+ * "allow_cidr" is false).
* note:
* network byte order assumed. this means 192.5.5.240/28 has
* 0b11110000 in its fourth octet.
@@ -71,7 +72,7 @@ const struct ares_in6_addr ares_in6addr_any = { { { 0, 0, 0, 0, 0, 0, 0, 0, 0,
* Paul Vixie (ISC), June 1996
*/
static int ares_inet_net_pton_ipv4(const char *src, unsigned char *dst,
- size_t size)
+ size_t size, ares_bool_t allow_cidr)
{
static const char xdigits[] = "0123456789abcdef";
static const char digits[] = "0123456789";
@@ -147,7 +148,8 @@ static int ares_inet_net_pton_ipv4(const char *src, unsigned char *dst,
}
bits = -1;
- if (ch == '/' && ares_isascii(src[0]) && ares_isdigit(src[0]) && dst > odst) {
+ if (allow_cidr && ch == '/' && ares_isascii(src[0]) && ares_isdigit(src[0]) &&
+ dst > odst) {
/* CIDR width specifier. Nothing can follow it. */
ch = *src++; /* Skip over the /. */
bits = 0;
@@ -317,7 +319,7 @@ static int ares_inet_pton6(const char *src, unsigned char *dst)
continue;
}
if (ch == '.' && ((tp + NS_INADDRSZ) <= endp) &&
- ares_inet_net_pton_ipv4(curtok, tp, NS_INADDRSZ) > 0) {
+ ares_inet_net_pton_ipv4(curtok, tp, NS_INADDRSZ, ARES_FALSE) > 0) {
tp += NS_INADDRSZ;
saw_xdigit = 0;
break; /* '\0' was seen by inet_pton4(). */
@@ -400,29 +402,23 @@ static int ares_inet_net_pton_ipv6(const char *src, unsigned char *dst,
SET_SOCKERRNO(EMSGSIZE);
return -1;
}
+ /* Only the first (bits+7)/8 bytes carry the network, zero the remainder of
+ * the caller's buffer so it never holds uninitialized data. */
+ memset(dst, 0, size);
memcpy(dst, &in6, bytes);
return (int)bits;
}
-/*
- * int
- * inet_net_pton(af, src, dst, size)
- * convert network number from presentation to network format.
- * accepts hex octets, hex strings, decimal octets, and /CIDR.
- * "size" is in bytes and describes "dst".
- * return:
- * number of bits, either imputed classfully or specified with /CIDR,
- * or -1 if some failure occurred (check errno). ENOENT means it was
- * not a valid network specification.
+/* Parse a network, not an address. See ares_inet_net_pton.h for the
+ * contract and how it differs from ares_inet_pton().
* author:
* Paul Vixie (ISC), June 1996
- *
*/
int ares_inet_net_pton(int af, const char *src, void *dst, size_t size)
{
switch (af) {
case AF_INET:
- return ares_inet_net_pton_ipv4(src, dst, size);
+ return ares_inet_net_pton_ipv4(src, dst, size, ARES_TRUE);
case AF_INET6:
return ares_inet_net_pton_ipv6(src, dst, size);
default:
@@ -430,22 +426,35 @@ int ares_inet_net_pton(int af, const char *src, void *dst, size_t size)
}
}
+/*
+ * int
+ * inet_pton(af, src, dst)
+ * convert an address (not a network) from presentation to network
+ * format, like POSIX inet_pton(). A "/bits" prefix length denotes a
+ * prefix, not an address (RFC 4291 Section 2.3 vs. the address text
+ * forms of Section 2.2), and is rejected; dst is always fully written
+ * on success (4 bytes for AF_INET, 16 bytes for AF_INET6).
+ * For a network with a prefix length, use ares_inet_net_pton().
+ * return:
+ * 1 on success, 0 if src is not a valid address for af (as POSIX
+ * inet_pton() does for any unparsable input), or -1 with errno set to
+ * EAFNOSUPPORT if af is not supported.
+ */
int ares_inet_pton(int af, const char *src, void *dst)
{
- int result;
- size_t size;
+ int result;
if (af == AF_INET) {
- size = sizeof(struct in_addr);
+ result =
+ ares_inet_net_pton_ipv4(src, dst, sizeof(struct in_addr), ARES_FALSE);
} else if (af == AF_INET6) {
- size = sizeof(struct ares_in6_addr);
+ result = ares_inet_pton6(src, dst);
} else {
SET_SOCKERRNO(EAFNOSUPPORT);
return -1;
}
- result = ares_inet_net_pton(af, src, dst, size);
- if (result == -1 && SOCKERRNO == ENOENT) {
- return 0;
- }
- return (result > -1) ? 1 : -1;
+ /* Neither parser can fail for a system reason: any failure (ENOENT, or
+ * EMSGSIZE for too many IPv4 octets as in "1.2.3.4.5") means src is not a
+ * valid address. */
+ return (result > -1) ? 1 : 0;
}
diff --git a/test/ares-test-internal.cc b/test/ares-test-internal.cc
index bf06a74605..294e3f4555 100644
--- a/test/ares-test-internal.cc
+++ b/test/ares-test-internal.cc
@@ -274,6 +274,23 @@ TEST_F(LibraryTest, InetNetPtoN) {
EXPECT_EQ(-1, ares_inet_net_pton(AF_INET, "253", &a4, sizeof(a4) - 1));
}
+// An IPv6 network with a prefix length only carries (bits+7)/8 bytes; the
+// rest of the destination must be zeroed rather than left as it was.
+TEST_F(LibraryTest, InetNetPtoNPrefixZeroFills) {
+ unsigned char a6[16];
+
+ memset(a6, 0x41, sizeof(a6));
+ EXPECT_EQ(0, ares_inet_net_pton(AF_INET6, "::1/0", a6, sizeof(a6)));
+ EXPECT_EQ("0000:0000:0000:0000:0000:0000:0000:0000",
+ AddressToString(a6, 16));
+
+ memset(a6, 0x41, sizeof(a6));
+ EXPECT_EQ(64, ares_inet_net_pton(AF_INET6, "2001:db8:1:2:3:4:5:6/64", a6,
+ sizeof(a6)));
+ EXPECT_EQ("2001:0db8:0001:0002:0000:0000:0000:0000",
+ AddressToString(a6, 16));
+}
+
TEST_F(LibraryTest, FreeLongChain) {
struct ares_addr_node *data = nullptr;
for (int ii = 0; ii < 100000; ii++) {
@@ -600,9 +617,46 @@ TEST_F(LibraryTest, InetPtoN) {
EXPECT_EQ(1, ares_inet_pton(AF_INET6, "12:34::ff", &a6));
EXPECT_EQ(1, ares_inet_pton(AF_INET6, "12:34::ffff:1.2.3.4", &a6));
EXPECT_EQ(0, ares_inet_pton(AF_INET, "xyzzy", &a4));
+ // Too many octets is unparsable input, not a system error.
+ EXPECT_EQ(0, ares_inet_pton(AF_INET, "1.2.3.4.5", &a4));
+ EXPECT_EQ(0, ares_inet_pton(AF_INET, "0x0102030405", &a4));
EXPECT_EQ(-1, ares_inet_pton(AF_INET+AF_INET6, "1.2.3.4", &a4));
}
+// ares_inet_pton() parses addresses, not networks: like inet_pton(3) it must
+// reject a "/bits" prefix length. It used to accept one and, for IPv6, write
+// only (bits+7)/8 bytes of dst while still returning 1.
+TEST_F(LibraryTest, InetPtoNRejectsPrefixLength) {
+ const char *cases6[] = { "::1/0", "ffff::1/16", "2001:db8::1/64",
+ "2001:db8::1/128", "::ffff:1.2.3.4/8",
+ "::ffff:1.2.3.4/32" };
+ const char *cases4[] = { "1.2.3.4/8", "1.2.3.4/0", "1.2.3.4/32", "10/8",
+ "0x0a000000/8" };
+ unsigned char a4[4];
+ unsigned char a6[16];
+ size_t i;
+
+ for (i = 0; i < sizeof(cases6) / sizeof(*cases6); i++) {
+ EXPECT_EQ(0, ares_inet_pton(AF_INET6, cases6[i], a6)) << cases6[i];
+ }
+ for (i = 0; i < sizeof(cases4) / sizeof(*cases4); i++) {
+ EXPECT_EQ(0, ares_inet_pton(AF_INET, cases4[i], a4)) << cases4[i];
+ }
+
+ // Without a prefix length, the whole destination is written.
+ memset(a6, 0x41, sizeof(a6));
+ EXPECT_EQ(1, ares_inet_pton(AF_INET6, "ffff::1", a6));
+ EXPECT_EQ("ffff:0000:0000:0000:0000:0000:0000:0001",
+ AddressToString(a6, 16));
+ memset(a6, 0x41, sizeof(a6));
+ EXPECT_EQ(1, ares_inet_pton(AF_INET6, "::ffff:1.2.3.4", a6));
+ EXPECT_EQ("0000:0000:0000:0000:0000:ffff:0102:0304",
+ AddressToString(a6, 16));
+ memset(a4, 0x41, sizeof(a4));
+ EXPECT_EQ(1, ares_inet_pton(AF_INET, "1.2.3.4", a4));
+ EXPECT_EQ("1.2.3.4", AddressToString(a4, 4));
+}
+
TEST_F(LibraryTest, FreeCorruptData) {
// ares_free_data(p) expects that there is a type field and a marker
// field in the memory before p. Feed it incorrect versions of each.
diff --git a/test/ares-test-mock-ai.cc b/test/ares-test-mock-ai.cc
index ae45c45527..43d0b89175 100644
--- a/test/ares-test-mock-ai.cc
+++ b/test/ares-test-mock-ai.cc
@@ -617,6 +617,29 @@ TEST_P(MockChannelTestAI, FamilyV6) {
EXPECT_THAT(result.ai_, IncludesV6Address("2121:0000:0000:0000:0000:0000:0000:0303"));
}
+// A numeric host with a "/bits" prefix length is a network, not an address.
+// It used to be accepted, returning uninitialized stack memory as the address.
+TEST_P(MockChannelTestAI, NumericHostRejectsPrefixLength) {
+ struct ares_addrinfo_hints hints = {0, 0, 0, 0};
+ hints.ai_family = AF_INET6;
+ hints.ai_flags = ARES_AI_NUMERICHOST;
+
+ AddrInfoResult result;
+ ares_getaddrinfo(channel_, "::1/0", NULL, &hints, AddrInfoCallback, &result);
+ Process();
+ EXPECT_TRUE(result.done_);
+ EXPECT_NE(ARES_SUCCESS, result.status_);
+
+ AddrInfoResult result2;
+ ares_getaddrinfo(channel_, "::1", NULL, &hints, AddrInfoCallback, &result2);
+ Process();
+ EXPECT_TRUE(result2.done_);
+ EXPECT_EQ(ARES_SUCCESS, result2.status_);
+ EXPECT_THAT(result2.ai_, IncludesNumAddresses(1));
+ EXPECT_THAT(result2.ai_,
+ IncludesV6Address("0000:0000:0000:0000:0000:0000:0000:0001"));
+}
+
// Test case for Issue #662
TEST_P(MockChannelTestAI, PartialQueryCancel) {
std::vector<byte> nothing;
@@ -0,0 +1,178 @@
From c7fa9ebcb0c0086730c53bf03191c03f03706246 Mon Sep 17 00:00:00 2001
From: "c-ares-backport-gha[bot]"
<299594594+c-ares-backport-gha[bot]@users.noreply.github.com>
Date: Tue, 20 Oct 2026 02:25:59 +0000
Subject: [PATCH] [Backport v1.34] ares_conn_read: don't drop UDP answers whose
source port isn't reported (#1371)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
# Description
Backport of #1370 to `v1.34`.
Signed-off-by: Brad House <brad@brad-house.com>
Co-authored-by: Brad House <brad@brad-house.com>
Upstream-Status: Backport [https://github.com/c-ares/c-ares/commit/c7fa9ebcb0c0086730c53bf03191c03f03706246]
GHSA: GHSA-fqh5-3x7m-wp44
---
docs/ares_set_socket_functions.3 | 20 ++++++++++++++++++++
src/lib/ares_conn.c | 9 ++++++++-
src/lib/ares_socket.c | 4 +++-
src/lib/ares_socket.h | 8 +++++---
test/ares-test-mock.cc | 32 +++++++++++++++++++++++++++++++-
5 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/docs/ares_set_socket_functions.3 b/docs/ares_set_socket_functions.3
index 605beb26ef..75c621cb48 100644
--- a/docs/ares_set_socket_functions.3
+++ b/docs/ares_set_socket_functions.3
@@ -149,6 +149,20 @@ a socket. Additional flags controlling behavior are in
address, the callback should fill this in. The \fIflags\fP parameter is
currently unused. See \fBrecvfrom(2)\fP.
+For UDP, the callback must fill in the full source address, including the
+source port, exactly as \fBrecvfrom(2)\fP does. c-ares discards a datagram
+whose source address does not match the server it queried, or whose source
+port does not match that server's port.
+
+\fBSecurity:\fP for compatibility with older callbacks, a reported source port
+of 0 is treated as unknown and only the address is checked. A callback that
+does not fill in the source port is therefore insecure: c-ares will accept
+answers sent from any port on the server's address, for example from another
+DNS server or any other service or user able to send UDP from that host, and
+those answers are cached and returned to the application. Such an
+implementation is non-compliant and should be fixed to report the real source
+port.
+
.TP 8
.B ares_ssize_t (*\fIasendto\fP)(ares_socket_t \fIfd\fP, const void * \fIbuffer\fP, size_t \fIlength\fP, int \fIflags\fP, const struct sockaddr * \fIaddress\fP, ares_socklen_t \fIaddress_len\fP, void * \fIuser_data\fP)
.br
@@ -305,6 +319,12 @@ Receives data from remote socket endpoint, if available. If the \fIaddr\fP
parameter is not NULL and the connection protocol provides the source address,
the callback should fill this in. See \fBrecvfrom(2)\fP
+For UDP, the callback must fill in the full source address, including the
+source port, as described above for \fIarecvfrom\fP in
+\fBares_set_socket_functions_ex\fP. \fBSecurity:\fP a callback that
+does not fill in the source port is insecure, and c-ares will accept answers
+sent from any port on the server's address.
+
.TP 8
.B ares_ssize_t (*\fIasendv\fP)(ares_socket_t \fIfd\fP, const struct iovec * \fIdata\fP, int \fIlen\fP, void * \fIuser_data\fP)
.br
diff --git a/src/lib/ares_conn.c b/src/lib/ares_conn.c
index b868053e62..7711d8649e 100644
--- a/src/lib/ares_conn.c
+++ b/src/lib/ares_conn.c
@@ -257,7 +257,14 @@ ares_conn_err_t ares_conn_read(ares_conn_t *conn, void *data, size_t len,
* functions might not filter on connect, so also check here that the
* datagram came from the server's address and UDP port. RFC 5452
* section 9.1 only requires the address to match; matching the port too
- * keeps one server from answering for another on the same address. */
+ * keeps one server from answering for another on the same address.
+ * Socket functions that fill in the source address but leave the port 0
+ * predate the port check, so for those only the address is checked. A
+ * sender cannot use this to skip the port check: the default socket
+ * functions connect() the socket, so the kernel drops a datagram from
+ * port 0, and an unprivileged process on the server's host cannot send
+ * from port 0, while an off-path attacker able to forge port 0 can forge
+ * the server's real port just as easily. */
if (err == ARES_CONN_ERR_SUCCESS &&
!ares_sockaddr_addr_port_eq((struct sockaddr *)&sa_storage,
&conn->server->addr,
diff --git a/src/lib/ares_socket.c b/src/lib/ares_socket.c
index 0a12935ab7..29d595e1a9 100644
--- a/src/lib/ares_socket.c
+++ b/src/lib/ares_socket.c
@@ -144,7 +144,9 @@ ares_bool_t ares_sockaddr_addr_port_eq(const struct sockaddr *sa,
sa_port = (CARES_INADDR_CAST(const struct sockaddr_in6 *, sa))->sin6_port;
}
- if (ntohs(sa_port) != port) {
+ /* A port of 0 means the source port was not reported, so only the address
+ * can be checked. */
+ if (sa_port != 0 && ntohs(sa_port) != port) {
return ARES_FALSE;
}
diff --git a/src/lib/ares_socket.h b/src/lib/ares_socket.h
index f7128e3493..1ede6987ee 100644
--- a/src/lib/ares_socket.h
+++ b/src/lib/ares_socket.h
@@ -151,13 +151,15 @@ ares_bool_t ares_sockaddr_addr_eq(const struct sockaddr *sa,
const struct ares_addr *aa);
/*! Compare the IP address and port of a socket address with an ares_addr and
- * port.
+ * port. A port of 0 in the socket address means the port is unknown, so it
+ * matches any port.
*
* \param[in] sa Socket address (AF_INET or AF_INET6)
* \param[in] aa Address to compare against
* \param[in] port Port to compare against, in host byte order
- * \return ARES_TRUE if the address families, IP addresses and ports are
- * equal, ARES_FALSE otherwise
+ * \return ARES_TRUE if the address families and IP addresses are equal and
+ * the ports are equal or the socket address port is 0, ARES_FALSE
+ * otherwise
*/
ares_bool_t ares_sockaddr_addr_port_eq(const struct sockaddr *sa,
const struct ares_addr *aa,
diff --git a/test/ares-test-mock.cc b/test/ares-test-mock.cc
index 058dc93001..cc5e6482f2 100644
--- a/test/ares-test-mock.cc
+++ b/test/ares-test-mock.cc
@@ -3364,6 +3364,7 @@ struct FakeSockData {
size_t query_len;
struct sockaddr_in peer;
unsigned short recv_port; /* network order; 0 reports the peer's port */
+ bool recv_no_port; /* report the peer's address with port 0 */
size_t recv_calls;
size_t bad_fd_calls;
size_t sock_state_bad;
@@ -3472,7 +3473,9 @@ static ares_ssize_t FakeSockRecvfrom(ares_socket_t sock, void *buffer,
if (address != NULL && address_len != NULL &&
*address_len >= (ares_socklen_t)sizeof(d->peer)) {
memcpy(address, &d->peer, sizeof(d->peer));
- if (d->recv_port != 0) {
+ if (d->recv_no_port) {
+ reinterpret_cast<struct sockaddr_in *>(address)->sin_port = 0;
+ } else if (d->recv_port != 0) {
reinterpret_cast<struct sockaddr_in *>(address)->sin_port = d->recv_port;
}
*address_len = sizeof(d->peer);
@@ -3750,6 +3753,33 @@ TEST_F(LibraryTest, UDPAnswerFromWrongPortDropped) {
ares_destroy(chan);
}
+// Socket functions written before the port check may fill in the source
+// address but leave the port 0. The address is still checked, but the answer
+// must not be dropped for its port.
+TEST_F(LibraryTest, UDPAnswerWithoutPortAccepted)
+{
+ ares_channel_t *chan = nullptr;
+ FakeSockData data;
+
+ memset(&data, 0, sizeof(data));
+ FakeSockInit(&chan, &data, 0, ARES_SOCKFUNC_FLAG_NONBLOCKING);
+
+ ares_query_dnsrec(chan, "www.google.com", ARES_CLASS_IN, ARES_REC_TYPE_A,
+ FakeSockQueryCallback, &data, NULL);
+ ASSERT_NE((size_t)0, data.query_len);
+ ASSERT_EQ(htons(53055), data.peer.sin_port);
+
+ data.recv_no_port = true;
+ data.recv_answer = true;
+ ares_process_fd(chan, 100, ARES_SOCKET_BAD);
+ EXPECT_FALSE(data.recv_answer);
+ EXPECT_TRUE(data.done);
+ // The fake answer carries no records.
+ EXPECT_EQ(ARES_ENODATA, data.status);
+ EXPECT_EQ((size_t)0, ares_queue_active_queries(chan));
+ ares_destroy(chan);
+}
+
// The close function is application code too. With no socket-state callback
// to carry the descriptor, it runs inline while ares_check_cleanup_conns()
// walks the server list and each server's connections, closing idle ones.
@@ -6,6 +6,14 @@ LICENSE = "MIT"
LIC_FILES_CHKSUM = "file://LICENSE.md;md5=d3e72a10e08191f2ca1be3f3228d78f3"
SRC_URI = "https://github.com/c-ares/c-ares/releases/download/v${PV}/${BPN}-${PV}.tar.gz \
file://0001-ares_cancel-ares_destroy-detach-queries-before-callbacks.patch \
file://0002-Deliver-deferred-socket-server-state-notifications.patch \
file://0003-ares_qcache-hand-callbacks-their-own-copy.patch \
file://0004-Handle-connections-torn-down-by-application.patch \
file://0005-ares_process-only-accept-an-answer-on-the-right-connection.patch \
file://0006-ares_save_options-read-channel-config-under-lock.patch \
file://0007-ares_inet_pton-reject-a-prefix-length.patch \
file://0008-ares_conn_read-dont-drop-UDP-answers-from-server-port.patch \
file://run-ptest"
SRC_URI[sha256sum] = "c222b6d681096f9444d2c4863d2c1174019e27cacca0a4a5c114d36dd7d7bf78"