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 edc9e55a5d856fa0cb5c3c0ecf7ef458b2b92a56..03135359d24bd7662c499741e348057da64146a4 100644 (file)
--- a/src/AuthUser.cci
+++ b/src/AuthUser.cci
@@ -47,8 +47,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

diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 24637822a9c8cb87702489a204cd2dcf5fe49a8f..20f766524eeacf11db3a08fb5df7f2f5c09112fc 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