From 306c32f50e289c401e4636976c97dc2b40fdd69b Mon Sep 17 00:00:00 2001
From: Timur Pocheptsov <timur.pocheptsov@qt.io>
Date: Tue, 28 Nov 2017 11:44:11 +0100
Subject: [PATCH] Fix out of bounds reads in qdnslookup_unix
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the response from res_nquery is too big for the buffer used to receive it
(of size PACKETSZ, a mere 512 bytes), the returned responseLength is the
size of the data that would have been delivered, had there been enough space.
Trying to process all of the data, including what wasn't delivered, leads to
reading past the end of the buffer, which either causes a crash or leads to
rubbish (from the stack) in the resulting QDnsRecords.  Easy to reproduce
using many long TXT records.

Replace the array with a QVarLengthArray; when the response is big, resize()
and retry, so as to actually get all of the data, so that we can process
it all. A follow-up patch will fix the case when even the second call/resize
buffer is not enough and we have to use TCP.

Task-number: QTBUG-64742
Change-Id: I173beb531e11a3828fd9c97f437afc192766035e
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
---
 src/network/kernel/qdnslookup_unix.cpp | 27 +++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/network/kernel/qdnslookup_unix.cpp b/src/network/kernel/qdnslookup_unix.cpp
index 1da00813ce2..ce1ec6442a6 100644
--- a/src/network/kernel/qdnslookup_unix.cpp
+++ b/src/network/kernel/qdnslookup_unix.cpp
@@ -42,6 +42,7 @@
 #if QT_CONFIG(library)
 #include <qlibrary.h>
 #endif
+#include <qvarlengtharray.h>
 #include <qscopedpointer.h>
 #include <qurl.h>
 #include <private/qnativesocketengine_p.h>
@@ -58,6 +59,8 @@
 #  include <gnu/lib-names.h>
 #endif
 
+#include <cstring>
+
 QT_BEGIN_NAMESPACE
 
 #if QT_CONFIG(library)
@@ -137,7 +140,7 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
 
     // Initialize state.
     struct __res_state state;
-    memset(&state, 0, sizeof(state));
+    std::memset(&state, 0, sizeof(state));
     if (local_res_ninit(&state) < 0) {
         reply->error = QDnsLookup::ResolverError;
         reply->errorString = tr("Resolver initialization failed");
@@ -189,11 +192,25 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
     QScopedPointer<struct __res_state, QDnsLookupStateDeleter> state_ptr(&state);
 
     // Perform DNS query.
-    unsigned char response[PACKETSZ];
-    memset(response, 0, sizeof(response));
-    const int responseLength = local_res_nquery(&state, requestName, C_IN, requestType, response, sizeof(response));
+    QVarLengthArray<unsigned char, PACKETSZ> buffer(PACKETSZ);
+    std::memset(buffer.data(), 0, buffer.size());
+    int responseLength = local_res_nquery(&state, requestName, C_IN, requestType, buffer.data(), buffer.size());
+    if (Q_UNLIKELY(responseLength > PACKETSZ)) {
+        buffer.resize(responseLength);
+        std::memset(buffer.data(), 0, buffer.size());
+        responseLength = local_res_nquery(&state, requestName, C_IN, requestType, buffer.data(), buffer.size());
+        if (Q_UNLIKELY(responseLength > buffer.size())) {
+            // Ok, we give up.
+            reply->error = QDnsLookup::ResolverError;
+            reply->errorString.clear(); // We cannot be more specific, alas.
+            return;
+        }
+    }
 
-    // Check the response header.
+    unsigned char *response = buffer.data();
+    // Check the response header. Though res_nquery returns -1 as a
+    // responseLength in case of error, we still can extract the
+    // exact error code from the response.
     HEADER *header = (HEADER*)response;
     const int answerCount = ntohs(header->ancount);
     switch (header->rcode) {