Bug 511615 (CVE-2009-3370)

Satchel should ignore untrusted events

RESOLVED FIXED in mozilla2.0



Form Manager
8 years ago
8 years ago


(Reporter: Paul Stone, Assigned: Dolske)



Bug Flags:
blocking1.9.2 +
blocking1.9.0.15 +
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta1-fixed, blocking1.9.1 .4+, status1.9.1 .4-fixed)


(Whiteboard: [sg:moderate])


(1 attachment, 1 obsolete attachment)



8 years ago
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.

Comment 1

8 years ago
Created attachment 395549 [details]

This has only been tested on Windows XP. It works more reliably if Firebug is not enabled for the page.

Comment 2

8 years ago
If you opened the testcase in a backgroud tab, you may have to refresh it to make it work correctly (due to focus issues)

Comment 3

8 years ago
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.
Ever confirmed: true
Whiteboard: [sg:moderate]

Comment 5

8 years ago
We should only show autocomplete in response to user-generated events.  But see also bug 381681 :(


8 years ago
Assignee: nobody → dolske

Comment 6

8 years ago
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.

Comment 7

8 years ago
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: location.bar → form.manager

Comment 8

8 years ago
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?(gavin.sharp)


8 years ago
Attachment #397395 - Attachment description: Patch v.1 (WIP) → Patch v.1

Comment 9

8 years ago
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.

Comment 10

8 years ago
Bah, this was nagging at me so I wrote a test today. Mostly done, will attach later.

Comment 11

8 years ago
Created attachment 398471 [details] [diff] [review]
Patch v.2

Now with tests!
Attachment #397395 - Attachment is obsolete: true
Attachment #398471 - Flags: review?(gavin.sharp)
Attachment #397395 - Flags: review?(gavin.sharp)

Comment 12

8 years ago
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?(gavin.sharp) → review+


8 years ago
Attachment #398471 - Flags: superreview?(mconnor)


8 years ago
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+
status1.9.1: --- → wanted
Flags: blocking1.9.0.15? → blocking1.9.0.15+

Comment 15

8 years ago
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!
Last Resolved: 8 years ago
status1.9.1: wanted → .4-fixed
status1.9.2: --- → final-fixed
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
status1.9.2: final-fixed → beta1-fixed

Comment 16

8 years ago
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.
Flags: blocking1.9.2? → blocking1.9.2+

Comment 17

8 years ago
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
Keywords: fixed1.9.0.15
Verified for 1.9.0 using test_bug_511615.html.
Keywords: fixed1.9.0.15 → verified1.9.0.15
Alias: CVE-2009-3370
Group: core-security


8 years ago
Blocks: 527935
You need to log in before you can comment on or make changes to this bug.