184618 - window.opener is not read-only and can be overwritten, which enables a user to close windows he did not open by changing the opener property

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Paul van Dam]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) Gecko/20021130

window.opener is not read-only and can be overwritten, which enables a user to 
close windows he did not open by changing the opener property. All versions of 
Mozilla and Internet Explorer have this flaw, so let Mozilla be the first to 
fix it ;). You can simply simulate it by doing (opener=self).close() for 
example. (Check the URL for a working example).

Reproducible: Always

Steps to Reproduce:
just execute the given js-code
Actual Results:  
Windows closed that should not have.

Expected Results:  
Give an error message about changing a read-only property. or ignore the fact 
and give an error about closing a window you did not open.

Comment 1

[Boris Zbarsky [:bzbarsky]]

To mitch.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, the js in question is simply:

(opener=self).close()

Comment 3

[Mitchell Stoltz (not reading bugmail)]

Cool...jst, would it be kosher to make this read-only (for content)?

Comment 4

[Johnny Stenback (:jst)]

This is a known problem. window.opener was settable in 4x too, but it was only
settable to null (see
http://lxr.mozilla.org/classic/source/lib/libmocha/lm_win.c#445). IOW, we should
do the same, the JS property window.opener should be settable only to null, at
least from untrusted code. The reason for null being a special case was IIRC
that in 4x you needed to set opener to null in order to let your opene's window
be collected while your window was open, IOW, if you open a window, that opens a
window, that opens a window and so on, even if you close the old windows and
just leave the newest one around, every window that was opened will still be in
memory due to the window.opener reference. In Mozilla the same doesn't apply, at
least not in the same way, due to the fact that window.opener is not a JS
property, it's a property with a getter/setter that sets a native reference
internally and that reference can be cleaned up when a window is closed.

So, IOW, let's let content code only set it to null in Mozilla too. But no
exception throwing, just fail silently if someone sets it to something other
than null.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

The problem is that setting it to null will allow the window to be closed by
script, and that's what we don't want. Do I need to add an extra bit of state
"PRBool wasOpenedByScript" apart from the opener property to control closing? Or
can we make "opener" completely non-settable (failing silently is OK).

Comment 6

[Alfonso Martinez]

*** Bug 196678 has been marked as a duplicate of this bug. ***

Comment 7

[Alfonso Martinez]

*** Bug 199980 has been marked as a duplicate of this bug. ***

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

It's well known hole in many browsers. Starting from IE, Opera, KHTML and
Mozilla too.

I think that opener should be not settable. I dont see any situation when it'd
be needed.

Comment 9

[Mitchell Stoltz (not reading bugmail)]

Opener is now settable only to null, which doesn't eliminate this problem. I'm
going to have to add another flag on the window to keep the opened-by-script state.

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Yes, it only eliminates secutiry hole. There is still posibility to loose
connection with parent window.

Is there any word in documentation of JS or ECMA about possiblity to overwrite
opener? I'm not sure if we should fix this. At least opener cannot be
overwritten by accident.

Comment 11

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - allow closing windows when opener has been nulled

This bug is basically fixed, except for one nit - if a script-opened window
sets its opener to null, it can't be closed by script. This patch remembers
that the window is script-opened and allows the close().

Comment 12

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 124852 [details] [diff] [review]
Patch - allow closing windows when opener has been nulled

Requesting reviews, but no hurry, this one isn't serious.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 124852 [details] [diff] [review]
Patch - allow closing windows when opener has been nulled

sr=jst

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.