Closed Bug 376987 Opened 17 years ago Closed 17 years ago

XSS using addEventListener


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: moz_bug_r_a4, Assigned: jst)


(Keywords: testcase, verified1.8.0.12, verified1.8.1.4, Whiteboard: [sg:high])


(2 files)

An attacker can add an event listener to a target site.  This allows an
attacker to perform an XSS attack as well as to sniff keystrokes (like bug

When a JS code calls addEventListener method, the native addEventListener
method is called through nsEventReceiverSH::AddEventListenerHelper unless the
JS code uses Components.lookupMethod to get addEventListener method.  And,
nsEventReceiverSH::AddEventListenerHelper does not check whether caller can
access a given event target.  Thus, an attacker can add an event listener to a
target site.

  w = open("target site");, "keypress", x, true);

And, when an event handler registered by the attacker is called, caller's
context is the target site, and the attacker can control a string conversion of
an event object.  Thus, the attacker can perform an XSS attack.
Assignee: dveditz → general
Component: Security → DOM
Keywords: testcase
OS: Windows XP → All
QA Contact: toolkit → ian
Hardware: PC → All
Whiteboard: [sg:high]
Since will be the last FF1.5 it'd be nice to squash this one in it.
Assignee: general → jst
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Attached patch Fix.Splinter Review
This fixes this bug by adding the missing security check to nsEventReceiverSH::AddEventListenerHelper().
Attachment #261180 - Flags: superreview?(bzbarsky)
Attachment #261180 - Flags: review?(mrbkap)
Attachment #261180 - Flags: review?(mrbkap) → review+
Comment on attachment 261180 [details] [diff] [review]

This looks pretty good, but should we also do an ACCESS_CALL_METHOD check here?  sr=bzbarsky with that added or an explanation of why it's not needed.
Attachment #261180 - Flags: superreview?(bzbarsky) → superreview+
bz, I could do a CALL_METHOD check in stead the GET_PROPERTY check, but I don't immediately see the need to do both checks. Sound good to you, or am I overlooking something something here?
They're equivalent modulo CAPS policies, but those could screw with this, I think.  Does XPConnect do both checks if this were not overridden via classinfo?  If so, that seems like the safe thing to do.
XPConnect would do both checks, I'll add the GET_PROPERTY check as well.
Same as above, with another check added to check that the caller can get the property addEventListener as well as call that method.
Attachment #261194 - Flags: superreview+
Attachment #261194 - Flags: review+
Attachment #261194 - Flags: approval1.8.1.4?
Attachment #261194 - Flags: approval1.8.0.12?
Fix checked in.
Closed: 17 years ago
Resolution: --- → FIXED
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
Comment on attachment 261194 [details] [diff] [review]
With GET_PROPERTY check too.

approved for and, a=dveditz for release-drivers.

Please wait to land this until much nearer code-freeze date since the patch makes it fairly obvious where to look for the problem.
Attachment #261194 - Flags: approval1.8.1.4?
Attachment #261194 - Flags: approval1.8.1.4+
Attachment #261194 - Flags: approval1.8.0.12?
Attachment #261194 - Flags: approval1.8.0.12+
Verified on both branches and trunk using the following builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070430 Firefox/
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070430 BonEcho/
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070428 Minefield/3.0a5pre
Group: security
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.