62029 - strncasecmp should be strcasecmp

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P3)

Description

[Stephen P. Morse]

The following line of code appears in nsSecureBrowserUIImpl.cpp.

    if ( PL_strncasecmp(host, "127.0.0.1",  9) == 0 ) {

Is there a reason for the strncasecmp rather than a straight strcasecmp as 
follows:

    if ( PL_strcasecmp(host, "127.0.0.1") == 0 ) {

Also there is a line

    if (PL_strncasecmp(password, (const char*)control->nonce.data,
                   control->nonce.len) == 0) {

Again, should that be a straight compare.  If it is a length compare, shouldn't 
the length be 5 instead of 9.

This code segment was copied to wallet.cpp as well, so if it is wrong it needs 
to be fixed in both files.

Comment 1

[Brendan Eich [:brendan]]

Nine lines down from the first PL_strncasecmp Steve cites is another one, of the
form PL_strncasecmp(password, secmanPassword, 9), which pretty clearly should be
PL_strcmp(password, secmanPassword) -- that is, neither case-independent nor
bounded by 9 chars!

/be

Comment 2

[John Unruh]

Reporter, is this still relevant now that PSM 2.0 is integrated into the nightly 
builds?

Comment 3

[Stephen P. Morse]

I'm not the one who can answer that since I don't know what you derived PSM2.0 
from.  If the file in question here is no longer used, and the comparabible file 
in PSM2.0 hasn't copied this bug, then this is no longer relevant.  The 
implementers of PSM2.0 are the ones who would know if this is still relevant or 
not.

Comment 4

[David P. Drinan]

PSM 2.0 is now part of the mozilla and commercial builds and has fixed alot of 
UI and ssl bugs that were in PSM 1.X. I'm doing a mass setting of bugs to 
be FIXED. If you believe that I've closed a bug in error, please re-open it. 
Thanks.

Comment 5

[John Unruh]

Verified.

Comment 6

[John Unruh]

Mass changing Security:Crypto to PSM

Comment 7

[John Unruh]

Mass changing Security:Crypto to PSM