Last Comment Bug 296514 - event handler and modal dialog allows XSS attacks
: event handler and modal dialog allows XSS attacks
Status: RESOLVED FIXED
[sg:fix] [cb] splitwindows?
: fixed1.7.13, fixed1.8
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
Mentors:
Depends on: splitwindows
Blocks: sbb? 298249
  Show dependency treegraph
 
Reported: 2005-06-03 06:29 PDT by shutdown
Modified: 2007-04-01 15:07 PDT (History)
8 users (show)
jaymoz: blocking1.7.9-
asa: blocking1.7.13+
chase: blocking‑aviary1.0.5-
chase: blocking‑aviary1.0.8+
cbeard: blocking1.8b3-
cbeard: blocking1.8b5+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (875 bytes, text/html)
2005-06-03 06:33 PDT, shutdown
no flags Details

Description shutdown 2005-06-03 06:29:10 PDT
User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050602 Firefox/1.0+ (2005060218)

Mozilla stops any scripts in the current page including event handlers
when it loads a new page. But in some cases, mozilla fails to stop event
handlers in the previous page. This is not a problem because such event
handler finish their exection before the new page is loaded. However,
if such event handler shows a modal dialog, the handler stops its
execution and mozilla continues to load the new page. If user closes
the modal dialog after mozilla loaded the new page, the handler
continues its execution in the context of the new page.

Reproducible: Always

Steps to Reproduce:
1. load the testcase.
2. push "invoke an exploit" button.

Actual Results:  
The event handler is executed in the context of the new page.

Expected Results:  
The event handler must be stopped before loading the new page.
Comment 1 shutdown 2005-06-03 06:33:26 PDT
Created attachment 185263 [details]
testcase

the testcase which tries to read cookies for www.google.com.
Comment 2 Bob Clary [:bc:] 2005-06-03 11:53:33 PDT
confirm it gets my google cookie.
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-06-03 20:20:22 PDT
We really need separate inner windows.... :(

Perhaps taking down a modal dialog in cases like this should throw an exception?
 That seems like the simplest way to abort script execution.
Comment 4 Brendan Eich [:brendan] 2005-06-03 20:41:20 PDT
We could validate cookie access using the script's principals, or some such.

An exception in this case seems like the easiest fix in the short run, and it
should not break anything.

bz: can you remind me whether the inner-window-object bug is on file? If not,
file away and cc: me.

/be
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-06-04 10:59:34 PDT
> We could validate cookie access using the script's principals, or some such.

Yes, but that feels like a stopgap... what other things would we need to do such
validation for?

In any case, that should be already happening.  The "cookie" property of
HTMLDocument is under the SameOrigin policy, so we're clearly ending up with the
wrong subject principal anyway... Or is that the needsSecurityCheck thing biting us?

I don't see an existing bug on the window split; I filed bug 296639 on that.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2005-06-15 18:00:57 PDT
I bet we're beeing bit by needsSecurityCheck again here, yeah. We need to either
fix that, or make modal dialogs suspend all network activity for the current
window while they're being shown. I tested that with just alert(), and that does
fix this bug, but there are other ways to bring up modal dialogs, so we need
more than what I hacked up so far (probably need to do this in our prompter
implementation). Thoughts?
Comment 7 Johnny Stenback (:jst, jst@mozilla.com) 2005-06-20 15:12:00 PDT
I was wrong above when betting that this was due to
documentNeedsSecurityCheck(). It's not. The problem is far worse than that.

What happens here is that we're running script from the previous page after the
new page is done loading. When we do that, and we access document.cookies
XPConnect does do the appropriate security check, but that check fails to notice
that the running code is from a different origin. Caps ends up calling
GetSubjectPrincipal(), which returns the principal of the new page, not the page
where the code came from. It does that by iterating over the JS stack frames and
it finds a cloned function on the stack (second frame, the first one is native,
so we skip it). Since it finds a clone, it can't rely on the principals compiled
into that function, so it walks up the function object's parent chain looking
for the principal... and since we've already loaded a new page, it finds the
principals of the new page.

I don't think we can change how the above works, so it seems like we need to
make modal dialogs suspend/resume network requests, at least for now...
Comment 8 Daniel Veditz [:dveditz] 2005-06-20 23:38:19 PDT
Michael Krax reports the same thing for xul in bug 298249. I assume it's a dupe,
but maybe just parallel implementations.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2005-06-23 18:08:29 PDT
I've got what's sort of a fix for this, but it's not complete yet. It'll fix
this problem, but I can still break the security model here by using more than
one window since the fix involves suspending/resuming requests in the parent
window when modal dialogs are opened, but if the parent is different than the
caller, then we've still got problems. Any number of windows could be used to
trigger a similar attack, so a complete fix that involves suspending/resuming
network requests is starting to look pretty nasty...
Comment 10 shutdown 2005-06-23 19:11:00 PDT
(In reply to comment #9)
> I've got what's sort of a fix for this, but it's not complete yet. It'll fix
> this problem, but I can still break the security model here by using more than
> one window since the fix involves suspending/resuming requests in the parent
> window when modal dialogs are opened, but if the parent is different than the
> caller, then we've still got problems. Any number of windows could be used to
> trigger a similar attack, so a complete fix that involves suspending/resuming
> network requests is starting to look pretty nasty...

bug 298315 has already been filed at 2005-06-21.
but it have not been confirmed yet.
Comment 11 Chase Phillips 2005-06-27 15:32:30 PDT
Per 1.0.5 meeting, minusing for 1.0.5, let's get this for 1.0.6.
Comment 12 Jay Patel [:jay] 2005-06-27 16:07:17 PDT
Updating the the 1.7.x flags.
Comment 13 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-08-11 11:10:34 PDT
Did split window fix this on the trunk?  If so, please mark this fixed.
Comment 14 shutdown 2005-08-12 01:57:30 PDT
The testcase no longer works. Fixed by bug 296639.
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b4) Gecko/20050811 Firefox/1.0+
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2005-09-13 17:17:27 PDT
Indeed, and it's fixed on the 1.8 branch too...
Comment 16 Daniel Veditz [:dveditz] 2006-02-06 19:32:25 PST
Fixed on the aviary1.0/mozilla1.7 branches by the split-window alternative (bug 316589)
Comment 17 Jay Patel [:jay] 2006-02-20 16:30:08 PST
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060220 Firefox/1.0.8, didn't see anything bad happen with testcase, just got an error in the jsc.

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