From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sun, 18 Nov 2012 10:37:19 +0000 (-0700)
Subject: Fix several buffer termination bugs
X-Git-Tag: SQUID_3_4_0_1~484
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0a84e4fbdaf9d33355297943776e37b242e3f15d;p=thirdparty%2Fsquid.git

Fix several buffer termination bugs

* strcpy() replaced in several places with strncpy() to ensure destination
  buffers are not overflowed.

* strncpy() does not nul-terminate the destination when the string being
  copied in exactly fills the buffer. Ensure we have terminated strings
  where it may matter.

 Detected by Coverity Scan. Issues 740309, 740310, 740311, 740481, 740483
---

diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc
index df2402fc94..8fdc201072 100644
--- a/src/AccessLogEntry.cc
+++ b/src/AccessLogEntry.cc
@@ -22,7 +22,7 @@ AccessLogEntry::getLogClientIp(char *buf, size_t bufsz) const
         if (tcpClient != NULL)
             tcpClient->remote.NtoA(buf, bufsz);
         else if (cache.caddr.IsNoAddr()) // e.g., ICAP OPTIONS lack client
-            strncpy(buf, "-", 1);
+            strncpy(buf, "-", bufsz);
         else
             cache.caddr.NtoA(buf, bufsz);
 }
diff --git a/src/dns_internal.cc b/src/dns_internal.cc
index fbb8704426..63a8b32104 100644
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -333,7 +333,8 @@ idnsAddPathComponent(const char *buf)
     }
 
     assert(npc < npc_alloc);
-    strcpy(searchpath[npc].domain, buf);
+    strncpy(searchpath[npc].domain, buf, sizeof(searchpath[npc].domain)-1);
+    searchpath[npc].domain[sizeof(searchpath[npc].domain)-1] = '\0';
     Tolower(searchpath[npc].domain);
     debugs(78, 3, "idnsAddPathComponent: Added domain #" << npc << ": " << searchpath[npc].domain);
     ++npc;
diff --git a/src/neighbors.cc b/src/neighbors.cc
index bbfe22ddb4..3cf71e04d8 100644
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -846,7 +846,7 @@ peerNoteDigestLookup(HttpRequest * request, CachePeer * p, lookup_t lookup)
 {
 #if USE_CACHE_DIGESTS
     if (p)
-        strncpy(request->hier.cd_host, p->host, sizeof(request->hier.cd_host));
+        strncpy(request->hier.cd_host, p->host, sizeof(request->hier.cd_host)-1);
     else
         *request->hier.cd_host = '\0';
 
diff --git a/src/tools.cc b/src/tools.cc
index b1f80a64e1..27d3773da7 100644
--- a/src/tools.cc
+++ b/src/tools.cc
@@ -1175,8 +1175,9 @@ parseEtcHosts(void)
             /* For IPV6 addresses also check for a colon */
             if (Config.appendDomain && !strchr(lt, '.') && !strchr(lt, ':')) {
                 /* I know it's ugly, but it's only at reconfig */
-                strncpy(buf2, lt, 512);
-                strncat(buf2, Config.appendDomain, 512 - strlen(lt) - 1);
+                strncpy(buf2, lt, sizeof(buf2)-1);
+                strncat(buf2, Config.appendDomain, sizeof(buf2) - strlen(lt) - 1);
+                buf1[sizeof(buf2)-1] = '\0';
                 host = buf2;
             } else {
                 host = lt;
diff --git a/src/url.cc b/src/url.cc
index 08eeaf0f08..d39a3275c9 100644
--- a/src/url.cc
+++ b/src/url.cc
@@ -313,10 +313,12 @@ urlParse(const HttpRequestMethod& method, char *url, HttpRequest *request)
         /* Is there any login information? (we should eventually parse it above) */
         t = strrchr(host, '@');
         if (t != NULL) {
-            strcpy((char *) login, (char *) host);
+            strncpy((char *) login, (char *) host, sizeof(login)-1);
+            login[sizeof(login)-1] = '\0';
             t = strrchr(login, '@');
             *t = 0;
-            strcpy((char *) host, t + 1);
+            strncpy((char *) host, t + 1, sizeof(host)-1);
+            host[sizeof(host)-1] = '\0';
         }
 
         /* Is there any host information? (we should eventually parse it above) */
diff --git a/src/wccp2.cc b/src/wccp2.cc
index 881ddac93a..e4cd968a34 100644
--- a/src/wccp2.cc
+++ b/src/wccp2.cc
@@ -605,7 +605,7 @@ wccp2_update_md5_security(char *password, char *ptr, char *packet, int len)
 
     SquidMD5Init(&M);
 
-    SquidMD5Update(&M, pwd, 8);
+    SquidMD5Update(&M, pwd, sizeof(pwd));
 
     SquidMD5Update(&M, packet, len);
 
@@ -650,7 +650,6 @@ wccp2_check_security(struct wccp2_service_list_t *srv, char *security, char *pac
 
     /* The password field, for the MD5 hash, needs to be 8 bytes and NUL padded. */
     memset(pwd, 0, sizeof(pwd));
-
     strncpy(pwd, srv->wccp_password, sizeof(pwd));
 
     /* Take a copy of the challenge: we need to NUL it before comparing */
@@ -660,7 +659,7 @@ wccp2_check_security(struct wccp2_service_list_t *srv, char *security, char *pac
 
     SquidMD5Init(&M);
 
-    SquidMD5Update(&M, pwd, 8);
+    SquidMD5Update(&M, pwd, sizeof(pwd));
 
     SquidMD5Update(&M, packet, len);