Last Comment Bug 380474 - XSS by loading a target site in the middle of calling addEventListener()
: XSS by loading a target site in the middle of calling addEventListener()
: fixed1.8.0.13, verified1.8.1.5
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: unspecified
: x86 Windows XP
-- normal (vote)
: mozilla1.9alpha6
Assigned To: Johnny Stenback (:jst,
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2007-05-11 22:04 PDT by moz_bug_r_a4
Modified: 2007-09-27 16:43 PDT (History)
11 users (show)
jonas: blocking1.9+
dveditz: blocking1.8.1.5+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.13+
dveditz: wanted1.8.0.x+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase 1 - a + QueryInterface() (1.29 KB, text/html)
2007-05-11 22:06 PDT, moz_bug_r_a4
no flags Details
testcase 2 - b + toString() (1.32 KB, text/html)
2007-05-11 22:07 PDT, moz_bug_r_a4
no flags Details
Fix. (966 bytes, patch)
2007-06-25 17:48 PDT, Johnny Stenback (:jst,
mrbkap: review+
brendan: superreview+
Details | Diff | Splinter Review
Branch version. (1.63 KB, patch)
2007-07-10 16:40 PDT, Johnny Stenback (:jst,
mrbkap: review+
jst: superreview+
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13+
Details | Diff | Splinter Review

Description User image moz_bug_r_a4 2007-05-11 22:04:39 PDT
A page load initiated in a function can be processed in the middle of that

  function f() {
    var d = w.document;
    w.location = url;
    w.document == d; // --> true
    alert(1); // or sync XMLHttpRequest, etc.
    w.document == d; // --> false

By using this behavior, an attacker can add an event listener to a target site. 
While addEventListener() is being called on a window, at the time of
conversions of arguments, it's possible to load a target site in the window
that addEventListener() is being called on.  When this happens, execution of
addEventListener() continues on the target site.

a) w.addEventListener(type, listener, ...);

In this case, type.toString() and listener.QueryInterface() are called in

b) Components.lookupMethod(w, "addEventListener")(type, listener, ...);

In this case, type.toString() and listener.QueryInterface() are called in
Comment 1 User image moz_bug_r_a4 2007-05-11 22:06:22 PDT
Created attachment 264571 [details]
testcase 1 - a + QueryInterface()

This works on trunk, fx2.0.0.x and fx1.5.0.x.
Comment 2 User image moz_bug_r_a4 2007-05-11 22:07:39 PDT
Created attachment 264572 [details]
testcase 2 - b + toString()

This works on trunk, fx2.0.0.x and fx1.5.0.x.
Comment 3 User image Johnny Stenback (:jst, 2007-06-25 17:48:36 PDT
Created attachment 269780 [details] [diff] [review]

This fixes both testcases in this bug by ensuring we use the inner window in the right places. The trick question here now is what other places do we need to do the same thing in...
Comment 4 User image Brendan Eich [:brendan] 2007-06-25 18:43:40 PDT
The problem could be phrased in terms of types and data flow:

1. Mark those methods that are allowed to run on an outer, which should be a small subset of all window methods, as "$outerok" or some such. In normal builds this macro expands to nothing.

2. Mark all the methods that must affect only an inner window as "$inneronly". We hope this is a small-ish subset too. We should not mark methods that already call OBJ_TO_INNER_OBJECT. Perhaps this marking could be automated: all window methods that don't call OBJ_TO_INNER_OBJECT and are not $outerok are by default $inneronly.

3. Run an Oink-based tool, not sure whether CQual++ could be used off-the-shelf, to find all flows from $outerok to $inneronly.

Taras, what do you think?

Comment 5 User image (dormant account) 2007-06-26 09:55:57 PDT
I think you cqual(or hacked variant of it) should be able to do this. As usual the hard part is specifying exactly what should be marked and how.
This doesn't need control flow, so it might be the first problem solvable by cqual.

1) Which methods qualify as window methods? Is there a class that contains them? Looks like you want to mark those with $outerok manually, correct?

2) Likewise, how do I obtain a list of those methods?

3) May be able to write a new tool so it does the marking and (maybe) even feeds it to cqual.
Comment 6 User image Johnny Stenback (:jst, 2007-06-27 17:41:08 PDT
Fix checked in. #2 is the million dollar question, in a way. I'm all for thorwing tools at this problem but it won't be trivial to get a reasonably correct list of what methods are ok to use inner/outer etc. We'll need a new bug on file for that work, I'm closing this bug for now.
Comment 7 User image Daniel Veditz [:dveditz] 2007-07-09 15:51:25 PDT
Will this patch work for the 1.8 branches? Do we need something else?
Comment 8 User image Johnny Stenback (:jst, 2007-07-09 16:42:17 PDT
This should work for branches.
Comment 9 User image Daniel Veditz [:dveditz] 2007-07-10 15:08:00 PDT
Comment on attachment 269780 [details] [diff] [review]

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

Please land ASAP!
Comment 10 User image Johnny Stenback (:jst, 2007-07-10 16:40:35 PDT
Created attachment 271764 [details] [diff] [review]
Branch version.

This is the same thing as the trunk version except that I can't use the macro OBJ_TO_INNER_OBJECT on the branches due to linking problems from it using internal JS engine functions that are not exported and reachable outside of the JS engine. So this patch expands the macro in the two places where I needed it and uses JS_GET_CLASS() instead of OBJ_GET_CLASS() which pulls in the dependency on the internal function.
Comment 11 User image Daniel Veditz [:dveditz] 2007-07-10 18:25:17 PDT
Comment on attachment 271764 [details] [diff] [review]
Branch version.

moving approval to correct patch.
Comment 12 User image Johnny Stenback (:jst, 2007-07-11 12:25:49 PDT
Fixed on branches.
Comment 13 User image juan becerra [:juanb] 2007-08-20 15:18:44 PDT
Does does not manifest itself in Tbird 15012 or 15013 (while going through Tbird 15013 bug verifications).
Comment 14 User image juan becerra [:juanb] 2007-08-20 15:21:47 PDT
Verified fixed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2007071317 Firefox/

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