Closed Bug 511639 Opened 10 years ago Closed 10 years ago

Login manager should ignore untrusted events

Categories

(Toolkit :: Password Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0
Tracking Status
status1.9.2 --- beta1-fixed
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed

People

(Reporter: smaug, Assigned: Dolske)

Details

(Keywords: verified1.9.0.15, verified1.9.1, Whiteboard: [sg:nse] not exploitable, we think.)

Attachments

(1 file)

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/src/nsLoginManager.js#323

I have no idea if there could be some security problems, but better be safe.
Marking security sensitive.

Would be good if someone familiar with the code could investigate if there are
any problems and if it would be ok to add listener for trusted events only.
Hmm. If content just generates a similar event at unexpected times, I think the worst that can happen is maybe getting it to fill in a password when signon.autofillForms has been disabled by the user (it's enabled by default).

However... Login manager does assume event.target is a document or HTML input. Can content create events with .target pointing to an arbitrary JS object? If it can, then it might be possible to make .target look like web content from another origin, and steal the passwords that login manger fills into it. That would be really bad. :(

There's certainly no reason to allow untrusted events here, so we should filter them anyway.
Hmm^2, wait, doesn't a chrome caller's addEventListener only listen to trusted events by default? And that's why if you want untrusted events you need to pass a 4th argument |true| to addEventListener to get them?

Maybe I'm missing something here.
IE, didn't bug 289940 prevent this?
(In reply to comment #2)
> Hmm^2, wait, doesn't a chrome caller's addEventListener only listen to trusted
> events by default?
Yes, when the listener is added to a chrome node.
nsLoginManager.js adds listeners to content nodes.
(In reply to comment #1)
> However... Login manager does assume event.target is a document or HTML input.
the .target points to the node or any descendant of the node to which the
listener was added.

> Can content create events with .target pointing to an arbitrary JS object?
No, AFAIK.
Attached patch Patch v.1Splinter Review
Assignee: nobody → dolske
Attachment #396901 - Flags: review?(Olli.Pettay)
Comment on attachment 396901 [details] [diff] [review]
Patch v.1

Ok, in this case this is the simplest way to not handle untrusted events.
Attachment #396901 - Flags: review?(Olli.Pettay) → review+
Attachment #396901 - Flags: review?(gavin.sharp)
It doesn't look to me like this would actually be exploitable, given comment 5. As long as .target is always a real DOM node, an evildoer would need to (1) get ahold of a DOM element for some other document, (2) dispatch the event to it, and (3) read the value from the node. I think existing security checks should prevent steps 1 and 3.
Attachment #396901 - Flags: review?(gavin.sharp) → review+
Attachment #396901 - Flags: superreview?(mconnor)
Attachment #396901 - Flags: superreview?(mconnor) → superreview+
Comment on attachment 396901 [details] [diff] [review]
Patch v.1

Approved for 1.9.1.4 and 1.9.0.15, a=dveditz
Attachment #396901 - Flags: approval1.9.2?
Attachment #396901 - Flags: approval1.9.1.4+
Attachment #396901 - Flags: approval1.9.0.15+
blocking1.9.1: --- → .4+
Flags: wanted1.9.0.x+
Flags: blocking1.9.2?
Flags: blocking1.9.0.15+
Attachment #396901 - Flags: approval1.9.2? → approval1.9.2+
Whiteboard: [sg:nse] not exploitable, we think.
Pushed http://hg.mozilla.org/mozilla-central/rev/909bac277b78
Pushed to 192: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0275e1852877
Pushed to 191: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a49b17a2144d
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Summary: nsLoginManager.js adds events listeners, which allow also untrusted events → Login manager should ignore untrusted events
Target Milestone: --- → mozilla1.9.3
Flags: blocking1.9.2? → blocking1.9.2+
Pushed to 190:

Checking in toolkit/components/passwordmgr/src/nsLoginManager.js;
  new revision: 1.33; previous revision: 1.32
Checking in toolkit/components/passwordmgr/test/test_basic_form_autocomplete.html;
  new revision: 1.8; previous revision: 1.7
Keywords: fixed1.9.0.15
Verified using checked in tests for 1.9.0 and 1.9.1.
Group: core-security
You need to log in before you can comment on or make changes to this bug.