summaryrefslogtreecommitdiffstats
path: root/src/network/kernel/qdnslookup_unix.cpp
diff options
context:
space:
mode:
authorTimur Pocheptsov <timur.pocheptsov@qt.io>2017-11-28 11:44:11 +0100
committerTimur Pocheptsov <timur.pocheptsov@qt.io>2017-12-01 15:05:41 +0000
commit306c32f50e289c401e4636976c97dc2b40fdd69b (patch)
tree8776008a1493536c06313de16c125b3d568f3029 /src/network/kernel/qdnslookup_unix.cpp
parentfbda8acc922217745bb3e7754d1cd450a0e0165a (diff)
Fix out of bounds reads in qdnslookup_unix
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>
Diffstat (limited to 'src/network/kernel/qdnslookup_unix.cpp')
-rw-r--r--src/network/kernel/qdnslookup_unix.cpp27
1 files changed, 22 insertions, 5 deletions
diff --git a/src/network/kernel/qdnslookup_unix.cpp b/src/network/kernel/qdnslookup_unix.cpp
index 1da00813ce..ce1ec6442a 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) {