From: Amos Jeffries <squid3@treenet.co.nz>
Date: Mon, 12 May 2008 02:22:52 +0000 (+1200)
Subject: Author: Finn Thain <fthain@telegraphics.com.au>
X-Git-Tag: SQUID_3_0_STABLE6~19
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0c251668bedf3aae025f4b966251cb4a9077c0a3;p=thirdparty%2Fsquid.git

Author: Finn Thain <fthain@telegraphics.com.au>
Bug 2323: basic auth leaks memory

- Several fixes resolving bad logic and leaks in basic auth.
---

diff --git a/src/AuthUser.cci b/src/AuthUser.cci
index 23da82652d..c1060cab1a 100644
--- a/src/AuthUser.cci
+++ b/src/AuthUser.cci
@@ -1,4 +1,3 @@
-
 /*
  * $Id: AuthUser.cci,v 1.3 2007/05/09 15:26:12 wessels Exp $
  *
@@ -34,6 +33,13 @@
  * Copyright (c) 2003, Robert Collins <robertc@squid-cache.org>
  */
 
+/* for assert() */
+#include "assert.h"
+/* for xstrdup() */
+#include "util.h"
+/* for safe_free() */
+#include "defines.h"
+
 char const *
 AuthUser::username () const
 {
@@ -43,8 +49,12 @@ AuthUser::username () const
 void
 AuthUser::username(char const*aString)
 {
-    assert (!username() || !aString);
-    username_ = aString ? xstrdup(aString) : NULL;
+    if (aString) {
+        assert(!username_);
+        username_ = xstrdup(aString);
+    } else {
+        safe_free(username_);
+    }
 }
 
 void
@@ -52,9 +62,7 @@ AuthUser::addRequest(AuthUserRequest *request)
 {
     /* lock for the request link */
 
-    lock()
-
-        ;
+    lock();
     dlink_node *node = dlinkNodeNew();
 
     dlinkAdd(request, node, &requests);
diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 3e6f655908..745b397cb3 100644
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -322,9 +322,7 @@ AuthBasicConfig::AuthBasicConfig()
 
 AuthBasicConfig::~AuthBasicConfig()
 {
-    if(basicAuthRealm)
-        delete basicAuthRealm;
-    basicAuthRealm = NULL;
+    safe_free(basicAuthRealm);
 }
 
 void
@@ -393,15 +391,20 @@ BasicUser::BasicUser(AuthConfig *config) : AuthUser (config) , passwd (NULL), cr
 bool
 BasicUser::decodeCleartext()
 {
-    char *sent_auth;
+    char *sent_auth = NULL;
+
     /* username and password */
     sent_auth = xstrdup(httpAuthHeader);
+
     /* Trim trailing \n before decoding */
     strtok(sent_auth, "\n");
 
     cleartext = uudecode(sent_auth);
 
-    xfree(sent_auth);
+    safe_free(sent_auth);
+
+    if (!cleartext)
+        return false;
 
     /*
      * Don't allow NL or CR in the credentials.
@@ -420,13 +423,19 @@ BasicUser::decodeCleartext()
 void
 BasicUser::extractUsername()
 {
-    char * tempusername = cleartext;
-    /* terminate the username string */
+    char * seperator = strchr(cleartext, ':');
 
-    if ((cleartext = strchr(tempusername, ':')) != NULL)
-        *(cleartext)++ = '\0';
+    if (seperator == NULL) {
+        username(cleartext);
+    } else {
+        /* terminate the username */
+        *seperator = '\0';
+
+        username(cleartext);
 
-    username (tempusername);
+        /* replace the colon so we can find the password */
+        *seperator = ':';
+    }
 
     if (!basicConfig.casesensitive)
         Tolower((char *)username());
@@ -435,22 +444,22 @@ BasicUser::extractUsername()
 void
 BasicUser::extractPassword()
 {
-    passwd = cleartext;
+    passwd = strchr(cleartext, ':');
 
-    if (cleartext == NULL) {
+    if (passwd == NULL) {
         debugs(29, 4, "authenticateBasicDecodeAuth: no password in proxy authorization header '" << httpAuthHeader << "'");
         passwd = NULL;
         currentRequest->setDenyMessage ("no password was present in the HTTP [proxy-]authorization header. This is most likely a browser bug");
-    } else if (*cleartext == '\0') {
-        debugs(29, 4, "authenticateBasicDecodeAuth: Disallowing empty password,user is '" << username() << "'");
-        passwd = NULL;
-        currentRequest->setDenyMessage ("Request denied because you provided an empty password. Users MUST have a password.");
+    } else {
+        ++passwd;
+        if (*passwd == '\0') {
+            debugs(29, 4, "authenticateBasicDecodeAuth: Disallowing empty password,user is '" << username() << "'");
+            passwd = NULL;
+            currentRequest->setDenyMessage ("Request denied because you provided an empty password. Users MUST have a password.");
+        } else {
+            passwd = xstrndup(passwd, USER_IDENT_SZ);
+        }
     }
-
-    if (passwd)
-        passwd = xstrndup(cleartext, USER_IDENT_SZ);
-
-    cleartext = NULL;
 }
 
 void