------------------------------------------------------------ revno: 12443 revision-id: squid3@treenet.co.nz-20130102100432-kunyl07x0klskupm parent: squid3@treenet.co.nz-20130102095944-qml95vbr8ryeoa0o author: Francesco Chemolli committer: Amos Jeffries branch nick: 3.3 timestamp: Wed 2013-01-02 03:04:32 -0700 message: 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 ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20130102100432-kunyl07x0klskupm # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # testament_sha1: 6eeffad6961c8cb9793a21801d17a6a75697d94a # timestamp: 2013-01-02 10:14:49 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.3 # base_revision_id: squid3@treenet.co.nz-20130102095944-\ # qml95vbr8ryeoa0o # # Begin patch === modified file 'lib/smblib/smblib.c' --- lib/smblib/smblib.c 2012-12-30 07:30:40 +0000 +++ lib/smblib/smblib.c 2013-01-02 10:04:32 +0000 @@ -252,7 +252,8 @@ 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 */ === modified file 'src/auth/digest/auth_digest.cc' --- src/auth/digest/auth_digest.cc 2013-01-02 04:18:02 +0000 +++ src/auth/digest/auth_digest.cc 2013-01-02 10:04:32 +0000 @@ -1026,7 +1026,7 @@ } } 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); === modified file 'src/client_side.cc' --- src/client_side.cc 2013-01-02 03:44:55 +0000 +++ src/client_side.cc 2013-01-02 10:04:32 +0000 @@ -2129,14 +2129,18 @@ } } -/** - * 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 @@ *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); === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2013-01-02 03:44:55 +0000 +++ src/client_side_request.cc 2013-01-02 10:04:32 +0000 @@ -1911,7 +1911,8 @@ #endif calloutContext->error->detailError(errDetail); calloutContext->readNextRequest = true; - c->expectNoForwarding(); + if (c != NULL) + c->expectNoForwarding(); doCallouts(); } //else if(calloutContext == NULL) is it possible? === modified file 'src/htcp.cc' --- src/htcp.cc 2012-09-22 14:21:59 +0000 +++ src/htcp.cc 2013-01-02 10:04:32 +0000 @@ -1180,14 +1180,13 @@ /* 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) { === modified file 'src/peer_proxy_negotiate_auth.cc' --- src/peer_proxy_negotiate_auth.cc 2012-09-01 08:17:17 +0000 +++ src/peer_proxy_negotiate_auth.cc 2013-01-02 10:04:32 +0000 @@ -331,8 +331,7 @@ 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 @@ 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 @@ 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 @@ 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 : "