XSS by loading a target site in the middle of calling addEventListener()

RESOLVED FIXED in mozilla1.9alpha6

Status

()

Core
DOM: Events
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: moz_bug_r_a4, Assigned: jst)

Tracking

({fixed1.8.0.13, verified1.8.1.5})

unspecified
mozilla1.9alpha6
x86
Windows XP
fixed1.8.0.13, verified1.8.1.5
Points:
---
Bug Flags:
blocking1.9 +
blocking1.8.1.5 +
wanted1.8.1.x +
blocking1.8.0.13 +
wanted1.8.0.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:high])

Attachments

(4 attachments)

(Reporter)

Description

10 years ago
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().
(Reporter)

Comment 1

10 years ago
Created attachment 264571 [details]
testcase 1 - a + QueryInterface()

This works on trunk, fx2.0.0.x and fx1.5.0.x.
(Reporter)

Comment 2

10 years ago
Created attachment 264572 [details]
testcase 2 - b + toString()

This works on trunk, fx2.0.0.x and fx1.5.0.x.
Flags: blocking1.9?
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Whiteboard: [sg:high]
Assignee: dveditz → jst
Component: Security → DOM: Events
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.9? → blocking1.9+

Updated

10 years ago
Target Milestone: --- → mozilla1.9alpha6
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
(Assignee)

Comment 3

10 years ago
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...
Attachment #269780 - Flags: superreview?(brendan)
Attachment #269780 - Flags: review?(mrbkap)

Updated

10 years ago
Attachment #269780 - Flags: review?(mrbkap) → review+

Updated

10 years ago
Attachment #269780 - Flags: superreview?(brendan) → superreview+
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

10 years ago
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.
(Assignee)

Comment 6

10 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Will this patch work for the 1.8 branches? Do we need something else?
(Assignee)

Comment 8

10 years ago
This should work for branches.
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!
Attachment #269780 - Flags: approval1.8.1.5+
Attachment #269780 - Flags: approval1.8.0.13+
(Assignee)

Comment 10

10 years ago
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.
Attachment #271764 - Flags: superreview+
Attachment #271764 - Flags: review?(mrbkap)
(Assignee)

Updated

10 years ago
Attachment #271764 - Flags: approval1.8.1.5?
Attachment #271764 - Flags: approval1.8.0.13?

Updated

10 years ago
Attachment #271764 - Flags: review?(mrbkap) → review+
Attachment #269780 - Flags: approval1.8.1.5+
Attachment #269780 - Flags: approval1.8.0.13+
Comment on attachment 271764 [details] [diff] [review]
Branch version.

moving approval to correct patch.
Attachment #271764 - Flags: approval1.8.1.5?
Attachment #271764 - Flags: approval1.8.1.5+
Attachment #271764 - Flags: approval1.8.0.13?
Attachment #271764 - Flags: approval1.8.0.13+
(Assignee)

Comment 12

10 years ago
Fixed on branches.
Keywords: fixed1.8.0.13, fixed1.8.1.5
Does does not manifest itself in Tbird 15012 or 15013 (while going through Tbird 15013 bug verifications).
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
Keywords: fixed1.8.1.5 → verified1.8.1.5
Flags: in-testsuite?
Group: security
You need to log in before you can comment on or make changes to this bug.