Closed Bug 511639 Opened 10 years ago Closed 10 years ago
Login manager should ignore untrusted events
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.
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 18.104.22.168 and 22.214.171.124, a=dveditz
Attachment #396901 - Flags: approval1.9.2? → approval1.9.2+
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
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
Verified using checked in tests for 1.9.0 and 1.9.1.
You need to log in before you can comment on or make changes to this bug.