The default bug view has changed. See this FAQ.

event handler and modal dialog allows XSS attacks

RESOLVED FIXED

Status

()

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

People

(Reporter: shutdown, Assigned: jst)

Tracking

({fixed1.7.13, fixed1.8})

Trunk
x86
Windows XP
fixed1.7.13, fixed1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.9 -
blocking1.7.13 +
blocking-aviary1.0.5 -
blocking-aviary1.0.8 +
blocking1.8b3 -
blocking1.8b5 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix] [cb] splitwindows?)

Attachments

(1 attachment)

875 bytes, text/html
Details
(Reporter)

Description

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

Comment 1

12 years ago
Created attachment 185263 [details]
testcase

the testcase which tries to read cookies for www.google.com.

Comment 2

12 years ago
confirm it gets my google cookie.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: events → jst
Flags: blocking1.8b3+
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
Whiteboard: [sg:fix]
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.
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
> 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.
Depends on: 296639
(Assignee)

Comment 6

12 years ago
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?
(Assignee)

Comment 7

12 years ago
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...
Michael Krax reports the same thing for xul in bug 298249. I assume it's a dupe,
but maybe just parallel implementations.
Blocks: 298249
(Assignee)

Comment 9

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

Comment 10

12 years ago
(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

12 years ago
Per 1.0.5 meeting, minusing for 1.0.5, let's get this for 1.0.6.
Flags: blocking-aviary1.0.6+
Flags: blocking-aviary1.0.5-
Flags: blocking-aviary1.0.5+

Comment 12

12 years ago
Updating the the 1.7.x flags.
Flags: blocking1.7.9-
Flags: blocking1.7.9+
Flags: blocking1.7.10+

Updated

12 years ago
Whiteboard: [sg:fix] → [sg:fix] [cb] bumped to 1.0.6, defer to 1.8b4?

Updated

12 years ago
Flags: blocking1.8b4+
Flags: blocking1.8b3-
Flags: blocking1.8b3+

Updated

12 years ago
Flags: blocking1.7.11+ → blocking1.7.12+
Did split window fix this on the trunk?  If so, please mark this fixed.
(Reporter)

Comment 14

12 years ago
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+
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 15

12 years ago
Indeed, and it's fixed on the 1.8 branch too...
Keywords: fixed1.8
Blocks: 256195

Updated

11 years ago
Flags: testcase+
Whiteboard: [sg:fix] [cb] bumped to 1.0.6, defer to 1.8b4? → [sg:fix] [cb] splitwindows?
Fixed on the aviary1.0/mozilla1.7 branches by the split-window alternative (bug 316589)
Keywords: fixed-aviary1.0.8, fixed1.7.13

Comment 17

11 years ago
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.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Group: security

Updated

10 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.