Closed Bug 511615 (CVE-2009-3370) Opened 11 years ago Closed 11 years ago

Satchel should ignore untrusted events


(Toolkit :: Form Manager, defect)

Not set



Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed


(Reporter: bugzilla, Assigned: Dolske)



(Keywords: verified1.9.0.15, Whiteboard: [sg:moderate])


(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv: 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: 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.
Attached file 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)
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.
Ever confirmed: true
Whiteboard: [sg:moderate]
We should only show autocomplete in response to user-generated events.  But see also bug 381681 :(
Assignee: nobody → dolske
Attached patch Patch v.1 (obsolete) — Splinter Review
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).
Component: Location Bar and Autocomplete → Form Manager
Product: Firefox → Toolkit
QA Contact: → form.manager
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.
Attachment #397395 - Flags: review?(
Attachment #397395 - Attachment description: Patch v.1 (WIP) → Patch v.1
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.
Attached patch Patch v.2Splinter Review
Now with tests!
Attachment #397395 - Attachment is obsolete: true
Attachment #398471 - Flags: review?(
Attachment #397395 - Flags: review?(
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.
Attachment #398471 - Flags: review?( → review+
Attachment #398471 - Flags: superreview?(mconnor)
Attachment #398471 - Flags: superreview?(mconnor) → superreview+
Comment on attachment 398471 [details] [diff] [review]
Patch v.2

belt AND suspenders?
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 and, a=dveditz
Attachment #398471 - Flags: approval1.9.2?
Attachment #398471 - Flags: approval1.9.1.4+
Attachment #398471 - Flags: approval1.9.0.15+
blocking1.9.1: --- → ?
Flags: in-testsuite?
Flags: blocking1.9.2?
Flags: blocking1.9.0.15?
Attachment #398471 - Flags: approval1.9.2? → approval1.9.2+
blocking1.9.1: ? → .4+
Flags: blocking1.9.0.15? → blocking1.9.0.15+
Pushed to 192:
Pushed to 191:

Paul Stone: Thanks for finding this!
Closed: 11 years ago
Flags: in-testsuite? → in-testsuite+
OS: Windows XP → All
Hardware: x86 → All
Resolution: --- → FIXED
Summary: Content can grab autocomplete history → Satchel should ignore untrusted events
Target Milestone: --- → mozilla1.9.3
Pushed test fix to 191:

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.
Flags: blocking1.9.2? → blocking1.9.2+
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/;
  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
Keywords: fixed1.9.0.15
Verified for 1.9.0 using test_bug_511615.html.
Alias: CVE-2009-3370
Group: core-security
You need to log in before you can comment on or make changes to this bug.