Last Comment Bug 344356 - Autofill username/password only fills the first form
: Autofill username/password only fills the first form
Status: RESOLVED FIXED
: fixed1.8.1.1
Product: Camino Graveyard
Classification: Graveyard
Component: HTML Form Controls (show other bugs)
: unspecified
: PowerPC Mac OS X
-- normal (vote)
: Camino1.5
Assigned To: Stuart Morgan
:
:
Mentors:
http://slashdot.org/
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-07-12 07:34 PDT by robbie_usenet
Modified: 2006-12-06 09:44 PST (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
fix (639 bytes, patch)
2006-11-30 21:11 PST, Stuart Morgan
stuart.morgan+bugzilla: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review

Description User image robbie_usenet 2006-07-12 07:34:21 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060712 Camino/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1b1) Gecko/20060712 Camino/1.0+

Slashdot redesigned their site to use better HTML/CSS. Since then, Camino doesn't want to autofill the username and login for the main page (although it accesses the keychain)

Reproducible: Always

Steps to Reproduce:
1. Create a Slashdot username/password
2. Login with your credentials
3. Logout of slashdot again
4. Go back to http://slashdot.org/

Actual Results:  
The slashdot main page is presented with a blank Username and Password field

Expected Results:  
It should have the Username and Password filled in with the details from step 2
Comment 1 User image Stuart Morgan 2006-07-12 08:38:57 PDT
If you open the disclosure triangle next to "Login" in the upper left, you'll find that it's filled in there.  Why they think they need two different login forms on the same page, I'm not sure.

I'm not sure there's much Camino can do here, except maybe change the password field detection to skip anything hidden.
Comment 2 User image Mike Pinkerton (not reading bugmail) 2006-07-12 14:40:13 PDT
agreed, hidden fields could be a security hole for sites that want to grab your password w/out you being able to see it.
Comment 3 User image Chris Lawson (gone) 2006-07-12 14:42:20 PDT
Confirming per IRC, since this is a potential security issue. We need to skip hidden form fields when looking for stuff to fill.

I'll hazard a guess that this won't be too difficult, and we should have it for 1.1.

cl
Comment 4 User image Chris Lawson (gone) 2006-07-12 17:02:46 PDT
Change needs to be made here:

http://lxr.mozilla.org/mozilla/source/camino/src/browser/KeychainService.mm#1125

That shouldn't return hidden fields.

cl
Comment 5 User image Samuel Sidler (old account; do not CC) 2006-07-12 17:14:15 PDT
(In reply to comment #2)
> agreed, hidden fields could be a security hole for sites that want to grab your
> password w/out you being able to see it.

Disagreed. If you intentionally give a site your username and password and save it, you want it to be filled in. We don't fill in username/passwords in iframes that don't match the domain, do we? Not as I understand it, at least. There's no security hole here.

The only bug I see is that we're not filling in both forms as we should be. 
Comment 6 User image Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-11-14 00:01:30 PST
Stuart, can you take this?
Comment 7 User image Stuart Morgan 2006-11-16 12:44:44 PST
(In reply to comment #5)
> The only bug I see is that we're not filling in both forms as we should be. 

Yeah, we stop once we've filled a form, and I don't think there's any compelling reason not to fill all the forms that look like login forms.

I also don't really buy the argument that a site would want to steal its own password from users--and if it were a real problem, skipping things in hidden divs wouldn't help, since I can think of a handful of other ways to get the same effect with varying degrees of undetectability to the browser and user.
Comment 8 User image Stuart Morgan 2006-11-21 21:19:41 PST
(In reply to comment #7)
> I also don't really buy the argument that a site would want to steal its own
> password from users

I wish I'd waited a few more days before making that claim. Anyway, I've filed bug 361463 to address that issue; regardless of how we solve that and what forms we don't fill, we should probably fill any legit forms on a page, even if there are more than one.
Comment 9 User image Stuart Morgan 2006-11-30 21:11:26 PST
Created attachment 247147 [details] [diff] [review]
fix

Pretty straightforward.
Comment 10 User image Stuart Morgan 2006-11-30 21:43:07 PST
Comment on attachment 247147 [details] [diff] [review]
fix

r=cl and kreeger per IRC
Comment 11 User image Mike Pinkerton (not reading bugmail) 2006-12-04 08:29:12 PST
Comment on attachment 247147 [details] [diff] [review]
fix

sr=pink
Comment 12 User image Stuart Morgan 2006-12-06 09:44:08 PST
Checked in on trunk and MOZILLA_1_8_BRANCH.

Note You need to log in before you can comment on or make changes to this bug.