From: wessels <>
Date: Mon, 23 Apr 2007 11:50:37 +0000 (+0000)
Subject: Fix coredump bug when auth header contains invalid characters
X-Git-Tag: SQUID_3_0_PRE6~55
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=35b3bc895f462f63b7165cf362a923c3ddaa2cd8;p=thirdparty%2Fsquid.git

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 2575b9e6ea..155299c61a 100644
--- 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 510d19fddd..e615186874 100644
--- 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;