From: Francesco Chemolli Date: Wed, 2 Jan 2013 10:04:32 +0000 (-0700) Subject: Various issues revealed by Coverty Scan X-Git-Tag: SQUID_3_3_0_3~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d03463f87dcf6429b808ef98c6dc41889e7844d5;p=thirdparty%2Fsquid.git Various issues revealed by Coverty Scan Also, improve documentation of parseHttpRequest. Address issues for the defect classes: - Array compared against 0 - Copy into fixed size buffer - Dereference after null check - Dereference before null check --- diff --git a/lib/smblib/smblib.c b/lib/smblib/smblib.c index a83c978671..79de3d6143 100644 --- a/lib/smblib/smblib.c +++ b/lib/smblib/smblib.c @@ -252,7 +252,8 @@ SMB_Handle_Type SMB_Connect(SMB_Handle_Type Con_Handle, SMBlib_errno = -SMBlibE_CallFailed; return NULL; } - strcpy(con -> desthost, host); + strncpy(con->desthost, host, sizeof(con->desthost)); + con->desthost[sizeof(con->desthost)-1]='\0'; /* Now connect to the remote end, but first upper case the name of the service we are going to call, sine some servers want it in uppercase */ diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc index efb6de6d68..efda8a7f84 100644 --- a/src/auth/digest/auth_digest.cc +++ b/src/auth/digest/auth_digest.cc @@ -1026,7 +1026,7 @@ Auth::Digest::Config::decode(char const *proxy_auth) } } else { /* cnonce and nc both require qop */ - if (digest_request->cnonce || digest_request->nc) { + if (digest_request->cnonce || digest_request->nc[0] != '\0') { debugs(29, 2, "missing qop!"); rv = authDigestLogUsername(username, digest_request); safe_free(username); diff --git a/src/client_side.cc b/src/client_side.cc index 4d57a35456..3555e9322e 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2129,14 +2129,18 @@ prepareTransparentURL(ConnStateData * conn, ClientHttpRequest *http, char *url, } } -/** - * parseHttpRequest() +/** Parse an HTTP request * - * Returns - * NULL on incomplete requests - * a ClientSocketContext structure on success or failure. - * Sets result->flags.parsed_ok to 0 if failed to parse the request. - * Sets result->flags.parsed_ok to 1 if we have a good request. + * \note Sets result->flags.parsed_ok to 0 if failed to parse the request, + * to 1 if the request was correctly parsed. + * \param[in] csd a ConnStateData. The caller must make sure it is not null + * \param[in] hp an HttpParser + * \param[out] mehtod_p will be set as a side-effect of the parsing. + * Pointed-to value will be set to Http::METHOD_NONE in case of + * parsing failure + * \param[out] http_ver will be set as a side-effect of the parsing + * \return NULL on incomplete requests, + * a ClientSocketContext structure on success or failure. */ static ClientSocketContext * parseHttpRequest(ConnStateData *csd, HttpParser *hp, HttpRequestMethod * method_p, HttpVersion *http_ver) @@ -2212,7 +2216,7 @@ parseHttpRequest(ConnStateData *csd, HttpParser *hp, HttpRequestMethod * method_ *method_p = HttpRequestMethod(&hp->buf[hp->req.m_start], &hp->buf[hp->req.m_end]+1); /* deny CONNECT via accelerated ports */ - if (*method_p == METHOD_CONNECT && csd && csd->port && csd->port->accel) { + if (*method_p == METHOD_CONNECT && csd->port && csd->port->accel) { debugs(33, DBG_IMPORTANT, "WARNING: CONNECT method received on " << csd->port->protocol << " Accelerator port " << csd->port->s.GetPort() ); /* XXX need a way to say "this many character length string" */ debugs(33, DBG_IMPORTANT, "WARNING: for request: " << hp->buf); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index c8e15981a4..364c113323 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1911,7 +1911,8 @@ ClientHttpRequest::handleAdaptationFailure(int errDetail, bool bypassable) #endif calloutContext->error->detailError(errDetail); calloutContext->readNextRequest = true; - c->expectNoForwarding(); + if (c != NULL) + c->expectNoForwarding(); doCallouts(); } //else if(calloutContext == NULL) is it possible? diff --git a/src/htcp.cc b/src/htcp.cc index 26c4ca6831..2c33a680c6 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -1180,14 +1180,13 @@ htcpHandleTstRequest(htcpDataHeader * dhdr, char *buf, int sz, Ip::Address &from /* s is a new object */ s = htcpUnpackSpecifier(buf, sz); - s->setFrom(from); - - s->setDataHeader(dhdr); - - if (NULL == s) { + if (s == NULL) { debugs(31, 3, "htcpHandleTstRequest: htcpUnpackSpecifier failed"); htcpLogHtcp(from, dhdr->opcode, LOG_UDP_INVALID, dash_str); return; + } else { + s->setFrom(from); + s->setDataHeader(dhdr); } if (!s->request) { diff --git a/src/peer_proxy_negotiate_auth.cc b/src/peer_proxy_negotiate_auth.cc index 1f926b4654..845e85320b 100644 --- a/src/peer_proxy_negotiate_auth.cc +++ b/src/peer_proxy_negotiate_auth.cc @@ -331,8 +331,7 @@ restart: p = strchr(buf, ':'); if (p) ++p; - if (keytab_filename) - xfree(keytab_filename); + xfree(keytab_filename); keytab_filename = xstrdup(p ? p : buf); } else { keytab_filename = xstrdup(kf); @@ -425,6 +424,10 @@ restart: mem_cache = (char *) xmalloc(strlen("FILE:/tmp/peer_proxy_negotiate_auth_") + 16); + if (!mem_cache) { + debugs(11, 5, "Error while allocating memory"); + return(1); + } snprintf(mem_cache, strlen("FILE:/tmp/peer_proxy_negotiate_auth_") + 16, "FILE:/tmp/peer_proxy_negotiate_auth_%d", (int) getpid()); @@ -432,6 +435,10 @@ restart: mem_cache = (char *) xmalloc(strlen("MEMORY:peer_proxy_negotiate_auth_") + 16); + if (!mem_cache) { + debugs(11, 5, "Error while allocating memory"); + return(1); + } snprintf(mem_cache, strlen("MEMORY:peer_proxy_negotiate_auth_") + 16, "MEMORY:peer_proxy_negotiate_auth_%d", (int) getpid()); @@ -439,8 +446,7 @@ restart: setenv("KRB5CCNAME", mem_cache, 1); code = krb5_cc_resolve(kparam.context, mem_cache, &kparam.cc); - if (mem_cache) - xfree(mem_cache); + xfree(mem_cache); if (code) { debugs(11, 5, HERE << "Error while resolving memory credential cache : "