]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Plugged memory leaks in digest authentication module
authorFrancesco Chemolli <kinkie@squid-cache.org>
Mon, 28 Jan 2013 05:39:19 +0000 (22:39 -0700)
committerAmos Jeffries <squid3@treenet.co.nz>
Mon, 28 Jan 2013 05:39:19 +0000 (22:39 -0700)
  Detected by Coverity Scan. Issue 740431.

src/auth/digest/auth_digest.cc

index b763efb1cacd7c4748d83b901f5883b0481d9c0c..2178098d23b7c7ba812f340c1907270f0c07b562 100644 (file)
@@ -933,10 +933,14 @@ Auth::Digest::Config::decode(char const *proxy_auth)
 
     /* 2069 requirements */
 
+    // return value.
+    Auth::UserRequest::Pointer rv;
     /* do we have a username ? */
     if (!username || username[0] == '\0') {
-        debugs(29, 2, HERE << "Empty or not present username");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Empty or not present username");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* Sanity check of the username.
@@ -944,33 +948,43 @@ Auth::Digest::Config::decode(char const *proxy_auth)
      * have been redone
      */
     if (strchr(username, '"')) {
-        debugs(29, 2, HERE << "Unacceptable username '" << username << "'");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Unacceptable username '" << username << "'");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* do we have a realm ? */
     if (!digest_request->realm || digest_request->realm[0] == '\0') {
-        debugs(29, 2, HERE << "Empty or not present realm");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Empty or not present realm");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* and a nonce? */
     if (!digest_request->nonceb64 || digest_request->nonceb64[0] == '\0') {
-        debugs(29, 2, HERE << "Empty or not present nonce");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Empty or not present nonce");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* we can't check the URI just yet. We'll check it in the
      * authenticate phase, but needs to be given */
     if (!digest_request->uri || digest_request->uri[0] == '\0') {
-        debugs(29, 2, HERE << "Missing URI field");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Missing URI field");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* is the response the correct length? */
     if (!digest_request->response || strlen(digest_request->response) != 32) {
-        debugs(29, 2, HERE << "Response length invalid");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Response length invalid");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* check the algorithm is present and supported */
@@ -978,8 +992,10 @@ Auth::Digest::Config::decode(char const *proxy_auth)
         digest_request->algorithm = xstrndup("MD5", 4);
     else if (strcmp(digest_request->algorithm, "MD5")
              && strcmp(digest_request->algorithm, "MD5-sess")) {
-        debugs(29, 2, HERE << "invalid algorithm specified!");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "invalid algorithm specified!");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* 2617 requirements, indicated by qop */
@@ -988,26 +1004,34 @@ Auth::Digest::Config::decode(char const *proxy_auth)
         /* check the qop is what we expected. */
         if (strcmp(digest_request->qop, QOP_AUTH) != 0) {
             /* we received a qop option we didn't send */
-            debugs(29, 2, HERE << "Invalid qop option received");
-            return authDigestLogUsername(username, digest_request);
+            debugs(29, 2, "Invalid qop option received");
+            rv = authDigestLogUsername(username, digest_request);
+            safe_free(username);
+            return rv;
         }
 
         /* check cnonce */
         if (!digest_request->cnonce || digest_request->cnonce[0] == '\0') {
-            debugs(29, 2, HERE << "Missing cnonce field");
-            return authDigestLogUsername(username, digest_request);
+            debugs(29, 2, "Missing cnonce field");
+            rv = authDigestLogUsername(username, digest_request);
+            safe_free(username);
+            return rv;
         }
 
         /* check nc */
         if (strlen(digest_request->nc) != 8 || strspn(digest_request->nc, "0123456789abcdefABCDEF") != 8) {
-            debugs(29, 2, HERE << "invalid nonce count");
-            return authDigestLogUsername(username, digest_request);
+            debugs(29, 2, "invalid nonce count");
+            rv = authDigestLogUsername(username, digest_request);
+            safe_free(username);
+            return rv;
         }
     } else {
         /* cnonce and nc both require qop */
         if (digest_request->cnonce || digest_request->nc) {
-            debugs(29, 2, HERE << "missing qop!");
-            return authDigestLogUsername(username, digest_request);
+            debugs(29, 2, "missing qop!");
+            rv = authDigestLogUsername(username, digest_request);
+            safe_free(username);
+            return rv;
         }
     }
 
@@ -1017,10 +1041,12 @@ Auth::Digest::Config::decode(char const *proxy_auth)
     nonce = authenticateDigestNonceFindNonce(digest_request->nonceb64);
     if (!nonce) {
         /* we couldn't find a matching nonce! */
-        debugs(29, 2, HERE << "Unexpected or invalid nonce received");
+        debugs(29, 2, "Unexpected or invalid nonce received");
         if (digest_request->user() != NULL)
             digest_request->user()->credentials(Auth::Failed);
-        return authDigestLogUsername(username, digest_request);
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     digest_request->nonce = nonce;
@@ -1028,8 +1054,10 @@ Auth::Digest::Config::decode(char const *proxy_auth)
 
     /* check that we're not being hacked / the username hasn't changed */
     if (nonce->user && strcmp(username, nonce->user->username())) {
-        debugs(29, 2, HERE << "Username for the nonce does not equal the username for the request");
-        return authDigestLogUsername(username, digest_request);
+        debugs(29, 2, "Username for the nonce does not equal the username for the request");
+        rv = authDigestLogUsername(username, digest_request);
+        safe_free(username);
+        return rv;
     }
 
     /* the method we'll check at the authenticate step as well */