I managed to find the source to firefox-1.5.dfsg+1.5.0.8 on a mirror
(http://mirror.xmu.edu.cn/archive.ubuntu.com/ubuntu/pool/main/f/firefox/;
it's already expired out of security.ubuntu.com and archive.ubuntu.com).

Diff between firefox-1.5.dfsg+1.5.0.8 and firefox-1.5.dfsg+1.5.0.9
reveals that a guard on userField being non-null was removed between
those two versions, viz:

-=- cut here -=-
@@ -941,20 +945,20 @@
     }
 
     if (firstMatch && !attachedToInput) {
-      nsAutoString buffer;
-
-      if (userField) {
+      AttachToInput(userField);
+      
+      if (prefillForm) {
+        nsAutoString buffer;
         if (NS_FAILED(DecryptData(firstMatch->userValue, buffer)))
           goto done;
-=- cut here -=- 

(Full diff of that file below.)

So since it appears to be fatal to call AttachToInput(NULL), it appears
that the function has been "deliberately" changed to cause Firefox to
crash when faced with a presaved form which has no username field.  This
seems to be undesirable.  At very worst it should refuse to fill in the
form and continue running; ideally it would continue the previous
Firefox behaviour and fill in the form without fuss.

There's nothing in the comments or changelog to explain why the guard on
userField being non-null was removed.  The entire changelog for .8 to .9
is:

-=- cut here -=-
firefox (1.5.dfsg+1.5.0.9-0ubuntu0.6.06) dapper-security; urgency=low

  * New upstream security update:
    - CVE-2006-6504, MFSA 2006-73: SVG Processing Remote Code Execution.
    - CVE-2006-6503, MFSA 2006-72: XSS by setting img.src to javascript: URI.
    - CVE-2006-6502, MFSA 2006-71: LiveConnect crash finalizing JS objects.
    - CVE-2006-6501, MFSA 2006-70: Privilege escallation using watch point.
    - CVE-2006-6497, CVE-2006-6498, CVE-2006-6499, MFSA 2006-68: Crashes
      with evidence of memory corruption.

 -- Kees Cook <[EMAIL PROTECTED]>  Tue,  2 Jan 2007 11:23:28 -0800
-=- cut here -=-

None of the CVE or MFSA documents referenced talk about the password
manager or saved password exploits or appear to explain why this code
was changed.  Given other security discussion I'm aware of I assume it's
part of an attempt to avoid the recently publicised password stealing
attack through user-created forms being pre-filled.

The list appears to come from here:

http://www.mozilla.org/projects/security/known-
vulnerabilities.html#firefox1.5.0.9

which is referenced from here:

http://www.mozilla.com/en-US/firefox/releases/1.5.0.9.html

which is the upstream announcement of 1.5.0.9.

Most of the change in nsPasswordManager.cpp appears to have come from
upstream (comparing diffs with and without the ubuntu dapper patches).
However  upstream 1.5.0.8 upstream also didn't have the guard on
userField being non-NULL.  That guard was being added by the ubuntu
dapper patch in 1.5.0.8, but is not being added in 1.5.0.9.

In particular we seem to have lost this patch:

-=- cut here -=-
* toolkit/components/passwordmgr/base/nsPasswordManager.cpp: Take patch
   from bz#235336 as suggested by Ian Jackson to allow password manager
   to work with sites that only have a password field, no username.
 -- Eric Dorland <[EMAIL PROTECTED]>  Mon,  6 Feb 2006 23:10:29 -0500
-=- cut here -=- 

I'm not sure what "bz#235336" is or where it can be found, but I
strongly suspect that it includes the guard on userField being non-null.

So it appears that a combination of an upstream change and dropping a patch 
that allowed password manager to work with password-only forms has caused
this crash.

It also appears to me that upstream will have this bug (crash with saved
passwords on forms with only a password field) if I'm reading their code
correctly.

Any chance we can have this "bz#235336" patch reapplied and/or the guard
on userField back again, so password manager works with password only forms and 
firefox doesn't crash?

Ewen

-=- full 1.5.0.8 to 1.5.0.9 diff for nsPasswordManager.cpp -=-
[EMAIL PROTECTED]:/var/tmp/ubuntu-dapper-firefox$ diff -u 
firefox-1.5.dfsg+1.5.0.{8,9}/toolkit/components/passwordmgr/base/nsPasswordManager.cpp
--- 
firefox-1.5.dfsg+1.5.0.8/toolkit/components/passwordmgr/base/nsPasswordManager.cpp
  2007-01-06 13:45:37.000000000 +1300
+++ 
firefox-1.5.dfsg+1.5.0.9/toolkit/components/passwordmgr/base/nsPasswordManager.cpp
  2007-01-06 12:53:22.000000000 +1300
@@ -815,6 +815,10 @@
 
   PRUint32 formCount;
   forms->GetLength(&formCount);
+  
+  // check to see if we should formfill.  failure is non-fatal
+  PRBool prefillForm = PR_TRUE;
+  mPrefBranch->GetBoolPref("prefillForms", &prefillForm);
 
   // We can auto-prefill the username and password if there is only
   // one stored login that matches the username and password field names
@@ -907,7 +911,7 @@
         continue;
       }
 
-      if (!oldUserValue.IsEmpty()) {
+      if (!oldUserValue.IsEmpty() && prefillForm) {
         // The page has prefilled a username.
         // If it matches any of our saved usernames, prefill the password
         // for that username.  If there are multiple saved usernames,
@@ -941,20 +945,20 @@
     }
 
     if (firstMatch && !attachedToInput) {
-      nsAutoString buffer;
-
-      if (userField) {
+      AttachToInput(userField);
+      
+      if (prefillForm) {
+        nsAutoString buffer;
         if (NS_FAILED(DecryptData(firstMatch->userValue, buffer)))
           goto done;
 
         userField->SetValue(buffer);
-        AttachToInput(userField);
-      }
 
-      if (NS_FAILED(DecryptData(firstMatch->passValue, buffer)))
-        goto done;
+        if (NS_FAILED(DecryptData(firstMatch->passValue, buffer)))
+          goto done;
 
-      passField->SetValue(buffer);
+        passField->SetValue(buffer);
+      }
     }
   }
 
-=- full 1.5.0.8 to 1.5.0.9 diff for nsPasswordManager.cpp -=-

-- 
Firefox: saved passwords causes crash with Mailman admin page
https://launchpad.net/bugs/77859

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to