summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThiago Macieira <thiago.macieira@intel.com>2023-05-11 21:40:15 -0700
committerQt Cherry-pick Bot <cherrypick_bot@qt-project.org>2023-05-16 04:12:42 +0000
commit55aee8697512af105dfefabc1e2ec41d4df1e45e (patch)
tree2c23d8e481a8f988c8f65f097ba3e512f8fffa07
parentb90c9b8655343d92e381a3004ec1f567d9905619 (diff)
QDnsLookup/Unix: make sure we don't overflow the bufferv6.5.16.5.1
The DNS Records are variable length and encode their size in 16 bits before the Record Data (RDATA). Ensure that both the RDATA and the Record header fields before it fall inside the buffer we have. Additionally reject any replies containing more than one query records. [ChangeLog][QtNetwork][QDnsLookup] Fixed a bug that could cause a buffer overflow in Unix systems while parsing corrupt, malicious, or truncated replies. Change-Id: I3e3bfef633af4130a03afffd175e4b9547654b95 Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io> Reviewed-by: Jani Heikkinen <jani.heikkinen@qt.io> (cherry picked from commit 7dba2c87619d558a61a30eb30cc1d9c3fe6df94c) Reviewed-by: Daniel Smith <Daniel.Smith@qt.io> (cherry picked from commit a2dc11b37fd71f785c342c40549f54edfdd1a6f8) Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r--src/network/kernel/qdnslookup_unix.cpp31
1 files changed, 25 insertions, 6 deletions
diff --git a/src/network/kernel/qdnslookup_unix.cpp b/src/network/kernel/qdnslookup_unix.cpp
index 75f7c6c440..de0113494f 100644
--- a/src/network/kernel/qdnslookup_unix.cpp
+++ b/src/network/kernel/qdnslookup_unix.cpp
@@ -193,7 +193,6 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
// 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) {
case NOERROR:
break;
@@ -226,18 +225,31 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
return;
}
- // Skip the query host, type (2 bytes) and class (2 bytes).
char host[PACKETSZ], answer[PACKETSZ];
unsigned char *p = response + sizeof(HEADER);
- int status = local_dn_expand(response, response + responseLength, p, host, sizeof(host));
- if (status < 0) {
+ int status;
+
+ if (ntohs(header->qdcount) == 1) {
+ // Skip the query host, type (2 bytes) and class (2 bytes).
+ status = local_dn_expand(response, response + responseLength, p, host, sizeof(host));
+ if (status < 0) {
+ reply->error = QDnsLookup::InvalidReplyError;
+ reply->errorString = tr("Could not expand domain name");
+ return;
+ }
+ if ((p - response) + status + 4 >= responseLength)
+ header->qdcount = 0xffff; // invalid reply below
+ else
+ p += status + 4;
+ }
+ if (ntohs(header->qdcount) > 1) {
reply->error = QDnsLookup::InvalidReplyError;
- reply->errorString = tr("Could not expand domain name");
+ reply->errorString = tr("Invalid reply received");
return;
}
- p += status + 4;
// Extract results.
+ const int answerCount = ntohs(header->ancount);
int answerIndex = 0;
while ((p < response + responseLength) && (answerIndex < answerCount)) {
status = local_dn_expand(response, response + responseLength, p, host, sizeof(host));
@@ -249,6 +261,11 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
const QString name = QUrl::fromAce(host);
p += status;
+
+ if ((p - response) + 10 > responseLength) {
+ // probably just a truncated reply, return what we have
+ return;
+ }
const quint16 type = (p[0] << 8) | p[1];
p += 2; // RR type
p += 2; // RR class
@@ -256,6 +273,8 @@ void QDnsLookupRunnable::query(const int requestType, const QByteArray &requestN
p += 4;
const quint16 size = (p[0] << 8) | p[1];
p += 2;
+ if ((p - response) + size > responseLength)
+ return; // truncated
if (type == QDnsLookup::A) {
if (size != 4) {