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 27578b0dafc80d12399367e966460b358d898495..b1f5cb21e76121e62a8a099b9ff856e030e5e41c 100644 (file)
--- a/src/dns_internal.cc
+++ b/src/dns_internal.cc
@@ -334,7 +334,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 73977af2f9aa77d637d512e45f3734c815512ca9..43aa5177200a32144d7cdd34d50ffbf10d276072 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 95e7ce6e13dfbf4d4cb4a4d0faad21228e664941..865d28cef83106dc62f35269545dea60cfb70d93 100644 (file)
--- a/src/tools.cc
+++ b/src/tools.cc
@@ -1135,8 +1135,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 760a887ddb89c5c532227ed3b5df9ca450024c34..e0f9d8c2ba47fb4c67041e8a6be8f47d585e2515 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);