diff options
author | William Pitcock <nenolod@dereferenced.org> | 2012-03-20 18:31:14 -0500 |
---|---|---|
committer | William Pitcock <nenolod@dereferenced.org> | 2012-03-20 18:31:14 -0500 |
commit | 9aa28f3730fb3dd69c1e06f78bb2bbc43d36c684 (patch) | |
tree | b903ed53ec9fdec2d801b4527a6be01c63f96d56 | |
parent | a6a07de0daa353bcd29056a4535a9c4784c113c8 (diff) |
dns: reject messages with lengths larger than DNSHeader with prejudice
This also includes when decompressing name entries.
-rw-r--r-- | src/dns.cpp | 43 |
1 files changed, 33 insertions, 10 deletions
diff --git a/src/dns.cpp b/src/dns.cpp index abdd0a54c..2bfa0be20 100644 --- a/src/dns.cpp +++ b/src/dns.cpp @@ -38,6 +38,8 @@ looks like this, walks like this or tastes like this. #include "configreader.h" #include "socket.h" +#define DN_COMP_BITMASK 0xC000 /* highest 6 bits in a DN label header */ + /** Masks to mask off the responses we get from the DNSRequest methods */ enum QueryInfo @@ -161,7 +163,10 @@ int CachedQuery::CalcTTLRemaining() /* Allocate the processing buffer */ DNSRequest::DNSRequest(DNS* dns, int rid, const std::string &original) : dnsobj(dns) { - res = new unsigned char[512]; + /* hardening against overflow here: make our work buffer twice the theoretical + * maximum size so that hostile input doesn't screw us over. + */ + res = new unsigned char[sizeof(DNSHeader) * 2]; *res = 0; orig = original; RequestTimeout* RT = new RequestTimeout(ServerInstance->Config->dns_timeout ? ServerInstance->Config->dns_timeout : 5, this, rid); @@ -690,9 +695,9 @@ DNSResult DNS::GetResult() /** A result is ready, process it */ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) { - unsigned i = 0; + unsigned i = 0, o; int q = 0; - int curanswer, o; + int curanswer; ResourceRecord rr; unsigned short ptr; @@ -717,7 +722,7 @@ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) /* Subtract the length of the header from the length of the packet */ length -= 12; - while ((unsigned int)q < header.qdcount && i < length) + while ((unsigned int)q < header.qdcount && i < (unsigned) length) { if (header.payload[i] > 63) { @@ -738,7 +743,7 @@ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) while ((unsigned)curanswer < header.ancount) { q = 0; - while (q == 0 && i < length) + while (q == 0 && i < (unsigned) length) { if (header.payload[i] > 63) { @@ -755,7 +760,7 @@ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) else i += header.payload[i] + 1; /* skip length and label */ } } - if (length - i < 10) + if ((unsigned) length - i < 10) return std::make_pair((unsigned char*)NULL,"Incorrectly sized DNS reply"); /* XXX: We actually initialise 'rr' here including its ttl field */ @@ -790,17 +795,31 @@ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) switch (rr.type) { + /* + * CNAME and PTR are compressed. We need to decompress them. + */ case DNS_QUERY_CNAME: - /* CNAME and PTR have the same processing code */ case DNS_QUERY_PTR: o = 0; q = 0; - while (q == 0 && i < length && o + 256 < 1023) + while (q == 0 && i < (unsigned) length && o + 256 < 1023) { + /* DN label found (byte over 63) */ if (header.payload[i] > 63) { memcpy(&ptr,&header.payload[i],2); - i = ntohs(ptr) - 0xC000 - 12; + + i = ntohs(ptr); + + /* check that highest two bits are set. if not, we've been had */ + if (!(i & DN_COMP_BITMASK)) + return std::make_pair((unsigned char *) NULL, "DN label decompression header is bogus"); + + /* mask away the two highest bits. */ + i &= ~DN_COMP_BITMASK; + + /* and decrease length by 12 bytes. */ + i =- 12; } else { @@ -813,7 +832,11 @@ DNSInfo DNSRequest::ResultIsReady(DNSHeader &header, int length) res[o] = 0; if (o != 0) res[o++] = '.'; - memcpy(&res[o],&header.payload[i + 1],header.payload[i]); + + if (o + header.payload[i] > sizeof(DNSHeader)) + return std::make_pair((unsigned char *) NULL, "DN label decompression is impossible -- malformed/hostile packet?"); + + memcpy(&res[o], &header.payload[i + 1], header.payload[i]); o += header.payload[i]; i += header.payload[i] + 1; } |