From: Francesco Chemolli Date: Fri, 28 Dec 2012 15:23:21 +0000 (+0100) Subject: Fix some issues revealed by Coverty Scan, improve documentation of parseHttpRequest X-Git-Tag: SQUID_3_4_0_1~412 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7830d88af82c371b86601e3a09f6464b53437f80;p=thirdparty%2Fsquid.git Fix some issues revealed by Coverty Scan, 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 7c7d424eeb..54dbdc4613 100644 --- a/src/auth/digest/auth_digest.cc +++ b/src/auth/digest/auth_digest.cc @@ -1025,7 +1025,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 54fcf1ab9d..bbcd1d8214 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2141,14 +2141,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) @@ -2224,7 +2228,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 == Http::METHOD_CONNECT && csd && csd->port && csd->port->accel) { + if (*method_p == Http::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 c1f395f3b3..2d3d9e5b29 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1968,7 +1968,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/forward.cc b/src/forward.cc index 4ed6f43ae0..171e0a4258 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -878,8 +878,10 @@ FwdState::sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &resp, Ssl::E const char *aReason = i->error_reason.empty() ? NULL : i->error_reason.c_str(); errDetails = new Ssl::ErrorDetail(i->error_no, peerCert.get(), brokenCert, aReason); } - delete check->sslErrors; - check->sslErrors = NULL; + if (check) { + delete check->sslErrors; + check->sslErrors = NULL; + } } if (!errs) diff --git a/src/htcp.cc b/src/htcp.cc index a671bb5f2a..b24ab0d980 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 : "