Fix coredump bug when auth header contains invalid characters

Apologies for the previous empty commit message.  Here is what
should have been there.

When a Proxy-Authorization header contains invalid characters (ie
NL or CR), BasicUser::extractUsername will emit an error message
and the username will not be set.  However, extractPassword was
also being called and it does not check for funny characters in the
cleartext string.  Thus passwd was being set.  BasicUser::valid was
returning true because it only looked at the passwd.

This bugfix has three components:

1) BasicUser::valid now checks the username, as well as the password.

2) Moved the check for invalid characters to BasicUser::decodeCleartext,
   which now returns boolean.  We will not call extractUsername and
   extractPassword if decodeCleartext fails.

3) extractUsername was freeing the cleartext string, but not setting
   the pointer to NULL, thus causing a double-free in the BasicUser
   destructor.

NOTE that the previous commit left the invalid character check in
extractUsername, but I realized it fits better in decodeCleartext
while writing the commit message, which led to the empty log.

diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 2575b9e6ea6db9df1bea19a1b493422e81e500dc..155299c61a8f2e1feccf2b34a585f2e8daa99be7 100644 (file)
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -1,5 +1,5 @@
 /*
- * $Id: auth_basic.cc,v 1.44 2007/04/23 05:43:08 wessels Exp $
+ * $Id: auth_basic.cc,v 1.45 2007/04/23 05:50:37 wessels Exp $
  *
  * DEBUG: section 29    Authenticator
  * AUTHOR: Duane Wessels
@@ -383,7 +383,7 @@ BasicUser::BasicUser(AuthConfig *config) : AuthUser (config) , passwd (NULL), cr
     flags.credentials_ok = 0;
 }
 
-void
+bool
 BasicUser::decodeCleartext()
 {
     char *sent_auth;
@@ -395,24 +395,25 @@ BasicUser::decodeCleartext()
     cleartext = uudecode(sent_auth);
 
     xfree(sent_auth);
-}
 
-bool
-BasicUser::extractUsername()
-{
     /*
      * Don't allow NL or CR in the credentials.
      * Oezguer Kesim <oec@codeblau.de>
      */
-    debug(29, 9) ("authenticateBasicDecodeAuth: cleartext = '%s'\n", cleartext);
+    debug(29, 9) ("BasicUser::decodeCleartext: '%s'\n", cleartext);
 
     if (strcspn(cleartext, "\r\n") != strlen(cleartext)) {
-        debug(29, 1) ("authenticateBasicDecodeAuth: bad characters in authorization header '%s'\n",
+        debug(29, 1) ("BasicUser::decodeCleartext: bad characters in authorization header '%s'\n",
                       httpAuthHeader);
         safe_free(cleartext);
         return false;
     }
+    return true;
+}
 
+void
+BasicUser::extractUsername()
+{
     char * tempusername = cleartext;
     /* terminate the username string */
 
@@ -423,7 +424,6 @@ BasicUser::extractUsername()
 
     if (!basicConfig.casesensitive)
         Tolower((char *)username());
-    return true;
 }
 
 void
@@ -454,9 +454,10 @@ BasicUser::decode(char const *proxy_auth, AuthUserRequest *auth_user_request)
 {
     currentRequest = auth_user_request;
     httpAuthHeader = proxy_auth;
-    decodeCleartext ();
-    if (extractUsername())
+    if (decodeCleartext ()) {
+       extractUsername();
        extractPassword();
+    }
     currentRequest = NULL;
     httpAuthHeader = NULL;
 }

diff --git a/src/auth/basic/auth_basic.h b/src/auth/basic/auth_basic.h
index 510d19fddd75dec8e9ecb93478bdd8a456bdb5f1..e615186874a3cd385bc2809d2b317dfacdeb97ea 100644 (file)
--- a/src/auth/basic/auth_basic.h
+++ b/src/auth/basic/auth_basic.h
@@ -71,8 +71,8 @@ unsigned int credentials_ok:
     BasicAuthQueueNode *auth_queue;
 
 private:
-    void decodeCleartext();
-    bool extractUsername();
+    bool decodeCleartext();
+    void extractUsername();
     void extractPassword();
     char *cleartext;
     AuthUserRequest *currentRequest;