From 768d2f25c2baf1cb05da0ef2e760c9f4b06140a3 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 25 Apr 2025 12:14:04 +0200 Subject: [PATCH] dnsdist: Fix an iterator out-of-bound read when removing a TCP-only server Introduced in https://github.com/PowerDNS/pdns/pull/15418 --- pdns/dnsdistdist/dnsdist-backend.cc | 3 +- .../test_PoolManagement.py | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 regression-tests.dnsdist/test_PoolManagement.py diff --git a/pdns/dnsdistdist/dnsdist-backend.cc b/pdns/dnsdistdist/dnsdist-backend.cc index 31e9d254cd..83b6fe7ba7 100644 --- a/pdns/dnsdistdist/dnsdist-backend.cc +++ b/pdns/dnsdistdist/dnsdist-backend.cc @@ -1078,6 +1078,7 @@ void ServerPool::removeServer(shared_ptr& server) bool tcpOnly = true; for (auto it = newServers->begin(); it != newServers->end();) { if (found) { + tcpOnly = tcpOnly && it->second->isTCPOnly(); /* we need to renumber the servers placed after the removed one, for Lua (custom policies) */ it->first = idx++; @@ -1087,10 +1088,10 @@ void ServerPool::removeServer(shared_ptr& server) it = newServers->erase(it); found = true; } else { + tcpOnly = tcpOnly && it->second->isTCPOnly(); idx++; it++; } - tcpOnly = tcpOnly && it->second->isTCPOnly(); } d_tcpOnly = tcpOnly; *servers = std::move(newServers); diff --git a/regression-tests.dnsdist/test_PoolManagement.py b/regression-tests.dnsdist/test_PoolManagement.py new file mode 100644 index 0000000000..9120d78dfb --- /dev/null +++ b/regression-tests.dnsdist/test_PoolManagement.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python +import dns +from dnsdisttests import DNSDistTest, pickAvailablePort + +class TestPoolManagement(DNSDistTest): + _config_template = """ + local backendPort = %d + server1 = newServer{address="127.0.0.1:"..backendPort, tcpOnly=true} + server2 = newServer{address="127.0.0.1:"..backendPort, tcpOnly=true} + server3 = newServer{address="127.0.0.1:"..backendPort, tcpOnly=true} + server1:addPool("new-pool") + server2:addPool("new-pool") + server3:addPool("new-pool") + server1:rmPool("new-pool") + server1:addPool("new-pool") + rmServer(server1) + rmServer(server2) + server3:rmPool("new-pool") + """ + + def testSimpleA(self): + """ + Pool management: A query without EDNS + """ + name = 'pool-mngmt.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN', use_edns=False) + response = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '127.0.0.1') + response.answer.append(rrset) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(response, receivedResponse) -- 2.47.2