dnsdist: Use a separate string for the DoH query and response

In rare cases, the responder thread might get a response _before_
the send() call of the DoH client thread has returned, resulting
in ASAN reporting a use-after-free since the response was written
into the now useless query object.

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index b8ab70b9b3f440cffa58e88d7ceab4c6ab1b401f..086642541309b9fa2ce79cabbb192927ffede52d 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -564,6 +564,7 @@ try {
 
         int oldFD = ids->origFD.exchange(-1);
         if (oldFD == origFD) {
+          ids->du = nullptr;
           /* we only decrement the outstanding counter if the value was not
              altered in the meantime, which would mean that the state has been actively reused
              and the other thread has not incremented the outstanding counter, so we don't
@@ -572,7 +573,6 @@ try {
         } else {
           du = nullptr;
         }
-        ids->du = nullptr;
 
         if(dh->tc && g_truncateTC) {
           truncateTC(response, &responseLen, responseSize, consumed);
@@ -595,7 +595,7 @@ try {
           if (du) {
 #ifdef HAVE_DNS_OVER_HTTPS
             // DoH query
-            du->query = std::string(response, responseLen);
+            du->response = std::string(response, responseLen);
             if (send(du->rsock, &du, sizeof(du), 0) != sizeof(du)) {
               delete du;
             }

diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc
index 8608922b91a4e2f064ee2de08ee314662e1015e6..c3c93511fa93c0b8b22892db14cfdbce4a273b1c 100644 (file)
--- a/pdns/dnsdistdist/doh.cc
+++ b/pdns/dnsdistdist/doh.cc
@@ -218,7 +218,7 @@ static int processDOHQuery(DOHUnit* du)
     }
 
     if (result == ProcessQueryResult::SendAnswer) {
-      du->query = std::string(reinterpret_cast<char*>(dq.dh), dq.len);
+      du->response = std::string(reinterpret_cast<char*>(dq.dh), dq.len);
       send(du->rsock, &du, sizeof(du), 0);
       return 0;
     }
@@ -595,8 +595,8 @@ static void on_dnsdist(h2o_socket_t *listener, const char *err)
     //    struct dnsheader* dh = (struct dnsheader*)du->query.c_str();
     //    cout<<"Attempt to send out "<<du->query.size()<<" bytes over https, TC="<<dh->tc<<", RCODE="<<dh->rcode<<", qtype="<<du->qtype<<", req="<<(void*)du->req<<endl;
 
-    du->req->res.content_length = du->query.size();
-    h2o_send_inline(du->req, du->query.c_str(), du->query.size());
+    du->req->res.content_length = du->response.size();
+    h2o_send_inline(du->req, du->response.c_str(), du->response.size());
   }
   else {
     switch(du->status_code) {

diff --git a/pdns/doh.hh b/pdns/doh.hh
index 7d411d25f8f1c3ac67b7b0da405ec0bdc497c3eb..f7c61c633597e79c822e15b8f608608ebed65592 100644 (file)
--- a/pdns/doh.hh
+++ b/pdns/doh.hh
@@ -55,6 +55,7 @@ struct st_h2o_req_t;
 struct DOHUnit
 {
   std::string query;
+  std::string response;
   ComboAddress remote;
   ComboAddress dest;
   st_h2o_req_t* req{nullptr};