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 a83c978671b0a02c428ec78b905d6265606b9f99..79de3d61436f01bad8aed9c4dc403571fa5b5c9b 100644 (file)
--- 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 efb6de6d682e5f5d95fb09ce351963797a53116b..efda8a7f842fbef8e4869e5a75da62d15d6fe98d 100644 (file)
--- 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 4d57a354561ce638c29584bd8be28995b5010098..3555e9322e2be3be6727858631747a233ce54622 100644 (file)
--- 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 c8e15981a452ce155e7906db6653b6d84b04106b..364c1133236f9ee9bd637e60340c5c4f8a3d1c19 100644 (file)
--- 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 26c4ca6831d537aac1bdc1ab34bed4b85074da61..2c33a680c63538f3c9248d1feb4bfb1b77c7498a 100644 (file)
--- 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 1f926b4654080b9afcde0e1f76f367d39724900e..845e85320b3967caee34a1ac90bac03f9c041cb7 100644 (file)
--- 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 : "