Last Comment Bug 376987 - XSS using addEventListener
: XSS using addEventListener
Status: VERIFIED FIXED
[sg:high]
: testcase, verified1.8.0.12, verified1.8.1.4
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-09 23:29 PDT by moz_bug_r_a4
Modified: 2007-06-12 10:52 PDT (History)
6 users (show)
dveditz: blocking1.8.1.4+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.12+
dveditz: wanted1.8.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix. (1.19 KB, patch)
2007-04-10 13:10 PDT, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
With GET_PROPERTY check too. (1.31 KB, patch)
2007-04-10 16:08 PDT, Johnny Stenback (:jst, jst@mozilla.com)
jst: review+
jst: superreview+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review

Description moz_bug_r_a4 2007-04-09 23:29:54 PDT
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] 2007-04-10 07:43:08 PDT
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, jst@mozilla.com) 2007-04-10 13:10:06 PDT
Created attachment 261180 [details] [diff] [review]
Fix.

This fixes this bug by adding the missing security check to nsEventReceiverSH::AddEventListenerHelper().
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2007-04-10 13:27:33 PDT
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, jst@mozilla.com) 2007-04-10 15:00:10 PDT
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 [:bz] (still a bit busy) 2007-04-10 15:03:30 PDT
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, jst@mozilla.com) 2007-04-10 16:06:37 PDT
XPConnect would do both checks, I'll add the GET_PROPERTY check as well.
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2007-04-10 16:08:15 PDT
Created attachment 261194 [details] [diff] [review]
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, jst@mozilla.com) 2007-04-10 17:07:51 PDT
Fix checked in.
Comment 10 Daniel Veditz [:dveditz] 2007-04-10 17:35:18 PDT
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) 2007-04-30 16:34:23 PDT
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

Note You need to log in before you can comment on or make changes to this bug.