From 654106b553d570fca540c94a65b98fb052971fc8 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 30 Nov 2012 06:34:49 -0700 Subject: [PATCH] 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 --- src/AccessLogEntry.cc | 2 +- src/dns_internal.cc | 3 ++- src/neighbors.cc | 2 +- src/tools.cc | 5 +++-- src/url.cc | 6 ++++-- src/wccp2.cc | 5 ++--- 6 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc index 31ea406864..b8d817dda8 100644 --- 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 3b7149f742..f721c7aa81 100644 --- 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 f6d6344ed7..79a330b2bd 100644 --- 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 c3f0f59fc6..2476d359fb 100644 --- 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 282fc5d73f..731ecd3838 100644 --- 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 1ee2ac53af..a5f6c8b5ce 100644 --- 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); -- 2.47.2