Bug 455584 - Use bz_crypt everywhere instead of the crypt() function

Patch by David Lawrence <dkl@redhat.com> = r/a=LpSolit

3 files changed, 25 insertions, 28 deletions

diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm
index f2c008dbf..0f73063d2 100644
--- a/Bugzilla/Auth/Verify/DB.pm
+++ b/Bugzilla/Auth/Verify/DB.pm
@@ -53,14 +53,9 @@ sub check_credentials {
         "SELECT cryptpassword FROM profiles WHERE userid = ?",
         undef, $user_id);
 
-    # Wide characters cause crypt to die
-    if (Bugzilla->params->{'utf8'}) {
-        utf8::encode($password) if utf8::is_utf8($password);
-    }
-
     # Using the internal crypted password as the salt,
     # crypt the password the user entered.
-    my $entered_password_crypted = crypt($password, $real_password_crypted);
+    my $entered_password_crypted = bz_crypt($password, $real_password_crypted);
  
     return { failure => AUTH_LOGINFAILED }
         if $entered_password_crypted ne $real_password_crypted;
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index f07228585..a8ba2d81c 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -464,21 +464,23 @@ sub file_mod_time {
 }
 
 sub bz_crypt {
-    my ($password) = @_;
-
-    # The list of characters that can appear in a salt.  Salts and hashes
-    # are both encoded as a sequence of characters from a set containing
-    # 64 characters, each one of which represents 6 bits of the salt/hash.
-    # The encoding is similar to BASE64, the difference being that the
-    # BASE64 plus sign (+) is replaced with a forward slash (/).
-    my @saltchars = (0..9, 'A'..'Z', 'a'..'z', '.', '/');
-
-    # Generate the salt.  We use an 8 character (48 bit) salt for maximum
-    # security on systems whose crypt uses MD5.  Systems with older
-    # versions of crypt will just use the first two characters of the salt.
-    my $salt = '';
-    for ( my $i=0 ; $i < 8 ; ++$i ) {
-        $salt .= $saltchars[rand(64)];
+    my ($password, $salt) = @_;
+
+    if (!defined $salt) {
+        # The list of characters that can appear in a salt.  Salts and hashes
+        # are both encoded as a sequence of characters from a set containing
+        # 64 characters, each one of which represents 6 bits of the salt/hash.
+        # The encoding is similar to BASE64, the difference being that the
+        # BASE64 plus sign (+) is replaced with a forward slash (/).
+        my @saltchars = (0..9, 'A'..'Z', 'a'..'z', '.', '/');
+
+        # Generate the salt.  We use an 8 character (48 bit) salt for maximum
+        # security on systems whose crypt uses MD5.  Systems with older
+        # versions of crypt will just use the first two characters of the salt.
+        $salt = '';
+        for ( my $i=0 ; $i < 8 ; ++$i ) {
+            $salt .= $saltchars[rand(64)];
+        }
     }
 
     # Wide characters cause crypt to die
@@ -489,6 +491,10 @@ sub bz_crypt {
     # Crypt the password.
     my $cryptedpassword = crypt($password, $salt);
 
+    # HACK: Perl has bug where returned crypted password is considered tainted
+    # Upstream Bug: http://rt.perl.org/rt3/Public/Bug/Display.html?id=59998
+    trick_taint($cryptedpassword) unless (is_tainted($password) || is_tainted($salt));
+
     # Return the crypted password.
     return $cryptedpassword;
 }
@@ -914,9 +920,10 @@ of the "mtime" parameter of the perl "stat" function.
 
 =over 4
 
-=item C<bz_crypt($password)>
+=item C<bz_crypt($password, $salt)>
 
 Takes a string and returns a C<crypt>ed value for it, using a random salt.
+An optional salt string may also be passed in.
 
 Please always use this function instead of the built-in perl "crypt"
 when initially encrypting a password.
diff --git a/userprefs.cgi b/userprefs.cgi
index 24a6a5699..4ce0f5715 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -92,12 +92,7 @@ sub SaveAccount {
 
         my $oldpassword = $cgi->param('Bugzilla_password');
 
-        # Wide characters cause crypt to die
-        if (Bugzilla->params->{'utf8'}) {
-            utf8::encode($oldpassword) if utf8::is_utf8($oldpassword);
-        } 
-
-        if (crypt($oldpassword, $oldcryptedpwd) ne $oldcryptedpwd) 
+        if (bz_crypt($oldpassword, $oldcryptedpwd) ne $oldcryptedpwd) 
         {
             ThrowUserError("old_password_incorrect");
         }