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()
Status: RESOLVED FIXED
[sg:high]
: 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, jst@mozilla.com)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks:
  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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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, jst@mozilla.com)
mrbkap: review+
brendan: superreview+
Details | Diff | Splinter Review
Branch version. (1.63 KB, patch)
2007-07-10 16:40 PDT, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
jst: superreview+
dveditz: approval1.8.1.5+
dveditz: approval1.8.0.13+
Details | Diff | Splinter Review

Description 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.

  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
nsEventReceiverSH::AddEventListenerHelper().

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

In this case, type.toString() and listener.QueryInterface() are called in
XPCWrappedNative::CallMethod().
Comment 1 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 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 Johnny Stenback (:jst, jst@mozilla.com) 2007-06-25 17:48:36 PDT
Created attachment 269780 [details] [diff] [review]
Fix.

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 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?

/be
Comment 5 (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 Johnny Stenback (:jst, jst@mozilla.com) 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 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 Johnny Stenback (:jst, jst@mozilla.com) 2007-07-09 16:42:17 PDT
This should work for branches.
Comment 9 Daniel Veditz [:dveditz] 2007-07-10 15:08:00 PDT
Comment on attachment 269780 [details] [diff] [review]
Fix.

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

Please land ASAP!
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 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 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 Johnny Stenback (:jst, jst@mozilla.com) 2007-07-11 12:25:49 PDT
Fixed on branches.
Comment 13 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 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:1.8.1.5) Gecko/2007071317 Firefox/2.0.0.5

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