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 df2402fc94273df2e64036a3e157a6d141959ece..8fdc201072648b8800377292212b478112368b0b 100644 (file)
--- 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 fbb87044264f502285e79fd0eb519e7dfb31fda3..63a8b32104909b9c50c6418a3e4c73dd7adc75eb 100644 (file)
--- 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 bbfe22ddb418d8bf3008cf6bc341ce33c1f86492..3cf71e04d811518b4d1d152b59effe37c5209f2d 100644 (file)
--- 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 b1f80a64e1364455ca9a0e84b6e8db1a8834de05..27d3773da7e3ebe04b8c03684b4b1d6a374d7df0 100644 (file)
--- 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 08eeaf0f08c6d6a087e6f3c4f5655d5a9c292922..d39a3275c94f7048705fc3cc91ae5e1e23fe9b55 100644 (file)
--- 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 881ddac93a27e571494516d44cc602166af1e0f3..e4cd968a34019dc0b35e6e57254e1a807dcbd6b8 100644 (file)
--- 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);