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 23da82652decca80f44b4ddb893abcba2dd3babe..c1060cab1aa0c1f1039ec8fbddf3489657cff254 100644 (file)
--- 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 3e6f6559084addd3d71e14e53e9436b34c73e616..745b397cb3eff955381f980c2f12088018e6bc41 100644 (file)
--- 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