376987 - XSS using addEventListener

Status: VERIFIED FIXED

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

Description

[moz_bug_r_a4]

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
18553).

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");
  document.addEventListener.call(w, "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.

Comment 2

[Daniel Veditz [:dveditz]]

Since 1.8.0.12 will be the last FF1.5 it'd be nice to squash this one in it.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Fix.

This fixes this bug by adding the missing security check to nsEventReceiverSH::AddEventListenerHelper().

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 261180 [details] [diff] [review]
Fix.

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.

Comment 5

[Johnny Stenback (:jst)]

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?

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Johnny Stenback (:jst)]

XPConnect would do both checks, I'll add the GET_PROPERTY check as well.

Comment 8

[Johnny Stenback (:jst)]

Attachment: With GET_PROPERTY check too.

Same as above, with another check added to check that the caller can get the property addEventListener as well as call that method.

Comment 9

[Johnny Stenback (:jst)]

Fix checked in.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 261194 [details] [diff] [review]
With GET_PROPERTY check too.

approved for 1.8.0.12 and 1.8.1.4, 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.

Comment 11

[Samuel Sidler (old account; do not CC)]

Verified on both branches and trunk using the following builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.12pre) Gecko/20070430 Firefox/1.5.0.12pre
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.4pre) Gecko/20070430 BonEcho/2.0.0.4pre
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a5pre) Gecko/20070428 Minefield/3.0a5pre