From 4a0a90263d213e0ff4abfe61d1628fb4971e5527 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Thu, 29 Nov 2012 03:26:58 -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 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 27578b0daf..b1f5cb21e7 100644 --- 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 73977af2f9..43aa517720 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 95e7ce6e13..865d28cef8 100644 --- 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 760a887ddb..e0f9d8c2ba 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); -- 2.47.2