diff options
author | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2017-11-28 11:44:11 +0100 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2017-12-01 15:05:41 +0000 |
commit | 306c32f50e289c401e4636976c97dc2b40fdd69b (patch) | |
tree | 8776008a1493536c06313de16c125b3d568f3029 /src/network/kernel/qdnslookup_unix.cpp | |
parent | fbda8acc922217745bb3e7754d1cd450a0e0165a (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.cpp | 27 |
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) { |