------------------------------------------------------------
revno: 12587
revision-id: squid3@treenet.co.nz-20130710124419-pnjuyz5wd20al66x
parent: squidadm@squid-cache.org-20130705030600-eqo6vu8vymjl420n
author: Nathan Hoad <nathan@getoffmalawn.com>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.3
timestamp: Wed 2013-07-10 06:44:19 -0600
message:
  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.
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20130710124419-pnjuyz5wd20al66x
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.3
# testament_sha1: 2f169d048e106c62aff3e9134fddb16c2cadbecf
# timestamp: 2013-07-10 12:49:52 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.3
# base_revision_id: squidadm@squid-cache.org-20130705030600-\
#   eqo6vu8vymjl420n
# 
# Begin patch
=== modified file 'src/dns_internal.cc'
--- src/dns_internal.cc	2012-11-29 10:26:58 +0000
+++ src/dns_internal.cc	2013-07-10 12:44:19 +0000
@@ -1667,23 +1667,29 @@
 void
 idnsALookup(const char *name, IDNSCB * callback, void *data)
 {
-    unsigned int i;
+    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;
+
+    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();
+
     int nd = 0;
-    idns_query *q;
-
-    if (idnsCachedLookup(name, callback, data))
-        return;
-
-    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)
+    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;