Protect against buffer overrun in DNS query generation

see SQUID-2013:2.

This bug has been present as long as the internal DNS component however
most code reaching this point is passing through URL validation first.
With Squid-3.2 Host header verification using DNS directly we may have
problems.

diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index 326547a3ef061647a95457024521d8dc3332838c..6a4e5b0b9959cdc70ae570952339ee9fae8215ae 100644 (file)
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -1716,23 +1716,29 @@ idnsSendSlaveAAAAQuery(idns_query *master)
 void
 idnsALookup(const char *name, IDNSCB * callback, void *data)
 {
-    unsigned int i;
-    int nd = 0;
-    idns_query *q;
+    size_t nameLength = strlen(name);
+
+    // Prevent buffer overflow on q->name
+    if (nameLength > NS_MAXDNAME) {
+        debugs(23, DBG_IMPORTANT, "SECURITY ALERT: DNS name too long to perform lookup: '" << name << "'. see access.log for details.");
+        callback(data, NULL, 0, "Internal error");
+        return;
+    }
 
     if (idnsCachedLookup(name, callback, data))
         return;
 
-    q = cbdataAlloc(idns_query);
+    idns_query *q = cbdataAlloc(idns_query);
     // idns_query is POD so no constructors are called after allocation
     q->xact_id.change();
     q->query_id = idnsQueryID();
 
-    for (i = 0; i < strlen(name); ++i)
+    int nd = 0;
+    for (unsigned int i = 0; i < nameLength; ++i)
         if (name[i] == '.')
             ++nd;
 
-    if (Config.onoff.res_defnames && npc > 0 && name[strlen(name)-1] != '.') {
+    if (Config.onoff.res_defnames && npc > 0 && name[nameLength-1] != '.') {
         q->do_searchpath = 1;
     } else {
         q->do_searchpath = 0;