User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:220.127.116.11) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:18.104.22.168) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729) A web page can trigger the autocomplete popup on a text field, and then generate 'cursor down' events to select a particular item from the drop down. When the user presses the left or right arrow keys, the textbox value will be set to the currently selected item. The web page can then grab the value. If a web page can convince a user to hold down or repeatedly press the left or right arrow keys, it can systematically grab all the data. I don't think this can be completely automated, since only user-generated left, right or enter key events cause the textbox to be filled. Depending on the 'name' attribute of the textbox, different sets of data can be grabbed. e.g. 'searchbar-history' will give the autocomplete data from the Firefox search bar. 'q' will give Google search queries. This could also be used to get usernames, email addresses and even credit card numbers (I have seen a number of shopping sites that don't set autocomplete="off" on their payment pages) Reproducible: Always Steps to Reproduce: See testcase (tested on Windows XP only) Actual Results: Synthesized key events can manipulate the autocomplete popup. Expected Results: Only user generated key events should trigger or affect autocomplete.
Created attachment 395549 [details] Testcase This has only been tested on Windows XP. It works more reliably if Firebug is not enabled for the page.
If you opened the testcase in a backgroud tab, you may have to refresh it to make it work correctly (due to focus issues)
Created attachment 395571 [details] Testcase (for Firefox trunk with 'awesomecomplete') The Firefox trunk has the new 'awesomecomplete' behaviour and makes this testcase work a little more efficiently. By typing a single 'space' character, the full autocomplete history is shown. For Firefox 3.5, a synthesized 'down key' or mouse click didn't trigger the full popup, but a single character does (e.g 'a' will show all entries starting with 'a'). So the first testcase has to loop through a-z0-9 to get the full results, which takes a bit longer.
Confirming. Setting to sg:moderate because user interaction is required.
We should only show autocomplete in response to user-generated events. But see also bug 381681 :(
Created attachment 397395 [details] [diff] [review] Patch v.1 This should fix the problem; I've added a check to all the event listener entry points to ignore untrusted events. The testcase in this bug no longer works, although I'd like to do a bit more testing and write a Mochitest for this. Most of the places were we were accepting trusted events were just stubs or otherwise not very interesting, but it seemed safer to just close off everything than try to figure out what events and (combinations of events) would be safe.
Moving to Form Mananger, since the generic autocomplete code doesn't seem to be involved (no calls to addEventListener).
Comment on attachment 397395 [details] [diff] [review] Patch v.1 I dunno if a testcase is worthwhile here... There's already test coverage to make sure we don't break normal usage. Testing this bug's fix, specifically, doesn't seem worthwhile because it requires a mix of privileged and unprivileged code, and I'm not very confident that it would reliably catch some future regressions of this type.
One further comment: The bug here is that unprivileged events were able to select arbitrary items from the autocomplete popup. The popup still seems to require a trusted event (ie, real user interaction) to actually select one of the items and fill it into the <input>. [I'm not sure what code is specifically enforcing that, though. The obvious places where arrows/enter trigger something happening don't seem to be involved with selecting an entry.] That seems to be why the original testcase/POC requires you to "hold down the right-arrow key", as it's that event that activates the selected entry. The testcase doesn't work if I modify it to generate fake right-arrow keypress events.
Bah, this was nagging at me so I wrote a test today. Mostly done, will attach later.
Created attachment 398471 [details] [diff] [review] Patch v.2 Now with tests!
Comment on attachment 398471 [details] [diff] [review] Patch v.2 >+ // try a keypress >+ case 14: >+ doKeyUnprivledged('v'); >+ nextTestPrivledged = true; >+ break; >+ case 15: >+ // XXX even with patch the popup is still opened. >+ // the keypress triggers a form input oninput / onchange >+ todo(false, "popup should be closed"); >+ checkPopupOpen(true, -1); This ends up looking complicated to fix... I tried moving the code in nsFormFillController::Input to ::KeyPress, but found that event fires before the form input's value has been updated. Making the satchel/autocomplete code deal with things in that state might be possible, but would be a fragile hack. That would be less ugly than trying to make the input event untrusted in some cases, though. It would be nice to seal this up tighter, but I don't think it's required to fix the issue in this bug.
Comment on attachment 398471 [details] [diff] [review] Patch v.2 unfortunately how to abuse this is pretty obvious from the patch -- we'll need to land everywhere in one bang, and want to catch the nearest available trains to release. Approved for 22.214.171.124 and 126.96.36.199, a=dveditz
Pushed http://hg.mozilla.org/mozilla-central/rev/909bac277b78 Pushed to 192: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b9eaa02ac19d Pushed to 191: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b89c79449fb4 Paul Stone: Thanks for finding this!
Pushed test fix to 191: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/15e78acc5aae These two subtests send spaces to the input, which is handled differently on 1.9.2 and trunk due to the landing of form awesomecomplete.
Pushed to 190 (patch + testfix): Checking in toolkit/components/satchel/src/nsFormFillController.cpp; new revision: 1.102; previous revision: 1.101 Checking in toolkit/components/satchel/src/nsFormFillController.h; new revision: 1.19; previous revision: 1.18 Checking in toolkit/components/satchel/test/Makefile.in; new revision: 1.3; previous revision: 1.2 Checking in toolkit/components/satchel/test/satchel_common.js; new revision: 1.2; previous revision: 1.1 Checking in toolkit/components/satchel/test/test_bug_511615.html; initial revision: 1.1
Verified for 1.9.0 using test_bug_511615.html.