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 31ea40686423cb8529ef02bd24be112fa55113e8..b8d817dda85ddb309ebd3890041b7a661d3a6ff7 100644 (file)
--- a/src/AccessLogEntry.cc
+++ b/src/AccessLogEntry.cc
@@ -13,7 +13,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 3b7149f742855792e0f49a377f98a2ab9da42b9b..f721c7aa81523647f182390443af146a3cc72e81 100644 (file)
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -324,7 +324,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 f6d6344ed757e1878056e9321c15ccd6561a2968..79a330b2bd9e6142fddf8f3bb121e9fb46ae78d7 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -833,7 +833,7 @@ peerNoteDigestLookup(HttpRequest * request, peer * 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 c3f0f59fc60ac72abd8329d8d0d691ce6807bb2f..2476d359fbd6f465466d3a51a7f43da3a4ea069d 100644 (file)
--- a/src/tools.cc
+++ b/src/tools.cc
@@ -1227,8 +1227,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);
+                buf2[sizeof(buf2)-1] = '\0';
                 host = buf2;
             } else {
                 host = lt;

diff --git a/src/url.cc b/src/url.cc
index 282fc5d73f647f8c137c7f7dad7391822743fd35..731ecd3838fe04b3b1bbf48c201a553c5e82a582 100644 (file)
--- a/src/url.cc
+++ b/src/url.cc
@@ -312,10 +312,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 1ee2ac53afaf5f377dee262981dfee56ecac17fa..a5f6c8b5ce2d6ad96a2a296a05a460da19b32233 100644 (file)
--- a/src/wccp2.cc
+++ b/src/wccp2.cc
@@ -613,7 +613,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);
 
@@ -659,7 +659,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 */
@@ -669,7 +668,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);