1003852 - Digest::SHA 5.82 and newer always croak on wide characters, preventing users with Unicode passwords from logging in

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, defect)

Description

[Alex Legler]

Sorry for the shameless copy-pasting of the summary, but this issue is basically bug 964113 applied to passwords. ;)

Trying to log in or change your password using a string containing wide characters results in an error when using Digest::SHA 5.820.0:
  Wide character in subroutine entry at /…/Bugzilla/Util.pm line 652.\n

The if part of the conditional block at that code location seems to have UTF encoding applied, while the else branch doesn't. The issue seems to be still present in trunk.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[Dylan Hardison [:dylan] (he/him)]

The right solution is to always encode (that is, pass bytes) to Digest (and crypt).
Checking utf8::is_isutf8() is against best practice. 

I believe the correct fix would be:

use Encode 'encode';
$password = encode('UTF-8', $password);

before the password is pased to Digest or crypt()

Comment 3

[Dylan Hardison [:dylan] (he/him)]

woops, sorry about changing those metadatas, I only meant to comment,

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8415318 [details] [diff] [review]
patch, v1

Review of attachment 8415318 [details] [diff] [review]:
-----------------------------------------------------------------

::: Bugzilla/Util.pm
@@ +626,5 @@
>      }
>  
> +    # Wide characters cause crypt and Digest to die.
> +    if (Bugzilla->params->{'utf8'}) {
> +        utf8::encode($password) if utf8::is_utf8($password);

I think calling is_utf8() here is against best practice for unicode in perl. If $password is not utf8 internally encoded, it just means it is values <127, and encode() would be a no-op anyway.

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8415318 [details] [diff] [review]
patch, v1

We can take up the is_utf8() practice in another bug.

Comment 6

[:glob ✱]

Comment on attachment 8415318 [details] [diff] [review]
patch, v1

r=glob

Comment 7

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   70e8ab7..d694e68  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   2234f68..1d6e06b  4.4 -> 4.4