252436 - non-ascii characters not recognized by master password dialog

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Walter Haidinger]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.7) Gecko/20040616
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-AT; rv:1.7) Gecko/20040616

If you set a master password having a non-ascii character (I've tested this with
the paragraph sign '§', 0xa7 or shift-3 on a German keyboard layout) in the
preferences, the password is not recognized in the popup master password dialog.

Luckily there is no bug in the preferences "Change master password" dialog, so
you can change your master password there and a reset is not required.


Reproducible: Always
Steps to Reproduce:
1. Goto: Edit->Preferences->Security->Master Password
   and set your master password to something including a non-ascii
   character, e.g. 'foo§bar' ('§' should be the paragraph sign, 0xa7, 
   or shift-3 on a German keyboard layout) and restart Mozilla.
2. Go to a page with a saved password to make the master password dialog
   appear. 
3. Enter 'foo§bar' as master password. It isn't recognized and your prompted
   over and over again. Even worse, Cancel does not work due to bug 237610.
Actual Results:  
The master password popup dialog did not recogzine my correctly entered master
password and prompted again and again.

Expected Results:  
accept the correctly entered master password?

Comment 1

[Josh Birnbaum]

*** Bug 198185 has been marked as a duplicate of this bug. ***

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

*** Bug 258569 has been marked as a duplicate of this bug. ***

Comment 3

[Matthias Versen [:Matti]]

*** Bug 277384 has been marked as a duplicate of this bug. ***

Comment 4

[Kai Engert [:KaiE:]]

mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp
function PK11PasswordPrompt

I guess the culprit is:
    char* str = ToNewCString(nsDependentString(password));

I suspect it must be changed to
    char* str = ToNewUTF8String(nsDependentString(password));

I need to get a mozilla build environment so I can try it out, hopefully later
today.

Comment 5

[Kai Engert [:KaiE:]]

Cool, my suggestion makes it indeed work, using a 123§123 password.

Will attach a patch once I get my CVS access back...

Comment 6

[Kai Engert [:KaiE:]]

Attachment: patch v1

Comment 7

[Kai Engert [:KaiE:]]

Comment on attachment 174157 [details] [diff] [review]
patch v1

David, could you please review? And possibly suggest a superreviewer, if
superreview is still required? Thanks.

Comment 8

[[e-mail]]

FYI nsPK11Token::ChangePassword uses NS_ConvertUCS2toUTF8

Comment 9

[Kai Engert [:KaiE:]]

Yes, but in this special situation we don't just require a temporary string that
can be passed on to a NSS function.

We need a string that is dynamically allocated and can be freed by a C function
from within the caller, so I guess using ToNewUTF8String is appropriate.

Comment 10

[Josh Birnbaum]

*** Bug 306393 has been marked as a duplicate of this bug. ***

Comment 11

[Stefan Breunig]

I guess this bug is a dupe of bug 237121 but since this one's got a patch the
older one should be marked as a dupe.

Comment 12

[Kai Engert [:KaiE:]]

*** Bug 237121 has been marked as a duplicate of this bug. ***

Comment 13

[Mike Shaver (:shaver emeritus)]

Comment on attachment 174157 [details] [diff] [review]
patch v1

sr=shaver

Comment 14

[Kai Engert [:KaiE:]]

fixed on trunk

Comment 15

[Josh Birnbaum]

*** Bug 316977 has been marked as a duplicate of this bug. ***

Comment 16

[OstGote!]

*** Bug 317506 has been marked as a duplicate of this bug. ***

Comment 17

[Christian Persch (GNOME) (away; not receiving bug mail)]

Comment on attachment 174157 [details] [diff] [review]
patch v1

Nominating for 1.8.0.1. The patch is an obviously-correct, zero risk fix for an i18n problem.

Comment 18

[OstGote!]

AFAIK 1.8.0.1 is only for security fixes, but we should try it. :-)

Comment 19

[Mike Schroepfer]

Approved to land on the 1.8 branch for 1.8.1/FF2 release.

Comment 20

[Kai Engert [:KaiE:]]

Checked in to MOZILLA_1_8_BRANCH

Should I set some fixed1.8.x keyword?

Comment 21

[Jesse Ruderman]

*** Bug 350152 has been marked as a duplicate of this bug. ***