Comment 5 for bug 77859

Revision history for this message
Ewen McNeill (ewen) wrote : Re: Firefox: saved passwords causes crash with Mailman admin page

Source for affected function that is segfaulting:

void
nsPasswordManager::AttachToInput(nsIDOMHTMLInputElement* aElement)
{
  nsCOMPtr<nsIDOMEventTarget> targ = do_QueryInterface(aElement);
  nsIDOMEventListener* listener = NS_STATIC_CAST(nsIDOMFocusListener*, this);

  targ->AddEventListener(NS_LITERAL_STRING("blur"), listener, PR_FALSE);
  targ->AddEventListener(NS_LITERAL_STRING("DOMAutoComplete"), listener, PR_FALSE);

  mAutoCompleteInputs.Put(aElement, 1);
}

(1956-1966 in firefox-1.5.dfsg+1.5.0.9/toolkit/components/passwordmgr/base/nsPasswordManager.cpp)

The affected line, 1962, is:

  targ->AddEventListener(NS_LITERAL_STRING("blur"), listener, PR_FALSE);

and appears to be segfaulting because the raw pointer inside targ is 0x0:

(gdb) print targ
$1 = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>}

I _think_ that the raw pointer inside targ is null because atElement is null,
but there's too much magic in the ns smart pointers to trace through without ever having seen the code before.

Working back up the stack, the caller is:

-=- cut here -=-
(gdb) up
#5 0xb67e7724 in nsPasswordManager::OnStateChange (this=0x89f6368,
    aWebProgress=0x86a6cec, aRequest=0x86939b4, aStateFlags=131088, aStatus=0)
    at nsPasswordManager.cpp:948
948 AttachToInput(userField);
(gdb) print userField
$6 = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>}
(gdb)
-=- cut here -=-

where the function contains the inline comment:

-=- cut here -=-
  // We can auto-prefill the username and password if there is only
  // one stored login that matches the username and password field names
  // on the form in question. Note that we only need to worry about a
  // single login per form.
-=- cut here -=-

All of which tends to confirm my impression that the problem is prefilling in a save password on forms which have only a password field and no username field,
Whatever changed in the code appears to no longer properly handle the situation where there is no username field and instead seems to assume that there'll be a username field to attach to if it finds a password field When it tries to attach to a non-existantant username field, thus dereferencing the null, it crashes.

Ewen