32571 - window.close() can close windows it doesn't own

Status: VERIFIED FIXED

(Core :: Security, defect, P2)

Description

[gaxzero]

javascript:window.close() in a frame created by a diffrent host is not suppose 
to close the windows unless the user agrees.
IE and Netscape do inform the user that the frame is attempting to close the 
window
However, Mozilla behaves exactly like i would like it to, because it someone 
trusts a host enought to include its files in a frame, it should also be allowed 
to do anything to it.  it is not the browsers job, but an agreement betreen the 
two hosts.
please try not to fix this bug.  it is too convenient for me 
see at http://go.to/gax

Comment 1

[Richard Zach]

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

Comment 2

[Richard Zach]

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

Comment 3

[rogerl (gone)]

Maybe DOM0, but seems like a security issue.

Comment 4

[Mitchell Stoltz (not reading bugmail)]

Bulk reassigning most of norris's bugs to mstoltz.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

This is worthwhile looking into. JS should NOT be able to close windows it
doesn't own. Marking M20 - to be looked at later.

Comment 6

[John Unruh]

Assigning QA to czhang

Comment 7

[Mitchell Stoltz (not reading bugmail)]

This is kinda bad, allows a script to quit the browser. Not damaging but
annoying. Recommend nsbeta3 but setting to P4, lower priority.

Comment 8

[leger]

Putting on [nsbeta3-] per PDT review today.  Other more critical work to do for 
nsbeta3 and rtm

Comment 9

[Mitchell Stoltz (not reading bugmail)]

In 4.7, if you call self.close(), a confirmation dialog appears. If a script
opens a window, then calls close() on it, no dialog appears.  Does anyone know
how 4.x determines when the confirmation should appear?

Adding 4xp keword as we're currently not asking the user at all. Should we be
asking? Or should we just add a security policy item "window.close on windows
not created by this script," set a default policy on that item, and let users
change it at will? Note that this policy is a bit more complex than same-origin,
as it has to distinguish windows created by script from windows opened by hand.
(most likely "self")

For PR3/rtm, presenting a dialog might be simpler. 

Comment 10

[Mitchell Stoltz (not reading bugmail)]

Future.

Comment 11

[Henrik Gemal]

Future?
I think this is potentialy really bad....

Comment 12

[Mitchell Stoltz (not reading bugmail)]

Henrik,
   Why is this really bad? Can you justify this statement? It doesn't lead to
any leakage or modification of information, it's merely an annoyance - an
annoying page can close your window, possibly exiting the browser. Is there
something else I'm missing here?

Comment 13

[Henrik Gemal]

Hmm.. It's properly not that bad, but I could think of sites that could be made 
so when you surf to the them, Mozilla suddenly quits.
But you're right "Not damaging but annoying....!"

Comment 14

[John Unruh]

Mass changing QA to ckritzer.

Comment 15

[Oded Arbel]

This behaviour seems to be a fix against bug <a
href"=http://bugzilla.mozilla.org/show_bug.cgi?id=9703">9703</a>. maybe that
"bug" can be put back into the code, as by its description I gather that its the
correct behaviour required ?
BTW - I think this behaviour is more the anoying - it's pretty damaging to
people who live on the web: closing a window you're browsing with will close
your browsing history, which some people, like me, rely upon to get back to
sites I like.

Comment 16

[Mitchell Stoltz (not reading bugmail)]

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

Comment 17

[Boris Zbarsky [:bzbarsky]]

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

Comment 18

[Jesse Ruderman]

IE always lets a top-level page close its window if the window was opened with
window.open().  This is useful for killing pop-up ads, because I can redirect
sites hosting pop-ups to a local web server that just serves pages that try to
close the window.  (If the window wasn't window.open()ed, IE asks the user if he
wants to allow the script to close the window.)

Assuming we follow IE in letting pages in window.open()ed windows close
themselves, we might as well let a frame close a window.opened window, since the
frame could replace the top-level window with
data:text/html,<script>window.close()</script> if it really wanted to close the
window.  (I haven't tested IE with frames.)

Comment 19

[Jesse Ruderman]

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

Comment 20

[Joseph Elwell]

Attachment: click here, and kiss your browser window goodbye.

Comment 21

[Joseph Elwell]

how uniquely annoying that websites that don't write javascript with Netscape 6
in mind can make your browser just disappear. (In my case the COOL doc page uses
frames and poorly written javascript which results in closure everytime).

Comment 22

[Mitchell Stoltz (not reading bugmail)]

Target is now 0.9.4, Priority P2.

Comment 23

[Jesse Ruderman]

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

Comment 24

[Mitchell Stoltz (not reading bugmail)]

time marches on...retargeting to 0.9.6

Comment 25

[Tom Mraz]

This can cause a BIG problem in our internet kiosk with embedded Mozilla. If a
malicious user goes to some page with such code, it either closes the browser
window (something which is really bad on such kind of machine) or (current
behavior) it somehow breaks the embedded mozilla so the Javascript never more
works and the embeded browser gets unstable (it's even worse than closing a
browser window).

Comment 26

[Matthias Versen [:Matti]]

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

Comment 27

[Mitchell Stoltz (not reading bugmail)]

Tom,
  As a temporary measure, before I get this fixed, you can disable all calls to
window.close() from JS by adding this line to prefs.js:

user_pref("capability.policy.default.Window.close","noAccess");

Comment 28

[Mitchell Stoltz (not reading bugmail)]

Moving the most time-critical bugs and minor security fixes to 0.9.7

Comment 29

[Martin Honnen]

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

Comment 30

[Martin Honnen]

Here are the NN4 docs that describe the intended behavior of the window.close method
  http://developer.netscape.com/docs/manuals/js/client/jsref/window.htm#1201822

Comment 31

[Mitchell Stoltz (not reading bugmail)]

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

Comment 32

[Mitchell Stoltz (not reading bugmail)]

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

Comment 33

[Tom Mraz]

It missed the 0.9.8 train. However nominating for 1.0 to not forget this bug.

Comment 34

[Mitchell Stoltz (not reading bugmail)]

0.9.9

Comment 35

[Boris Zbarsky [:bzbarsky]]

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

Comment 36

[Johnny Stenback (:jst)]

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

Comment 37

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - add pref and confirmation dialog

Comment 38

[Johnny Stenback (:jst)]

Comment on attachment 69560 [details] [diff] [review]
Patch - add pref and confirmation dialog

sr=jst

Comment 39

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 69560 [details] [diff] [review]
Patch - add pref and confirmation dialog

r=heikki

Just check what 4.x asks and see if it makes sense to use the exact wording in
the dialog.

Comment 40

[Chris McAfee]

mstoltz patch backed out.

Comment 41

[/\/\arcio Galli]

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

Comment 42

[/\/\arcio Galli]

Nominating nsbeta1 because this is important for end users. window.close without
security alert is not cool. 

This also have impact when a plugin performs javascript:window.close. For
example a flash plugin can close the window easily (see the above comment dup). 

Comment 43

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9.9

Comment 44

[Navin Gupta]

looks like partial checkins were backed out. remaining checkins caused 
regression bug 127938

Comment 45

[Jesse Ruderman]

Are the pref and dialog necessary?  I don't see a need for either.

Comment 46

[Mitchell Stoltz (not reading bugmail)]

Let's try to finish this for mozilla1.0

Comment 47

[/\/\arcio Galli]

adding topembed, we don't want to have Gecko powered products closing the
document window easily. :)

Comment 48

[scottputterman]

ADT2 per ADT triage.

Comment 49

[Dave Barrowman]

EDT plussing - future embedding clients should have this.

Comment 50

[Boris Zbarsky [:bzbarsky]]

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

Comment 51

[Fabian Guisset]

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

Comment 52

[Boris Zbarsky [:bzbarsky]]

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

Comment 53

[Leland Wang]

I think this bug is as important as Bug 103452

Comment 54

[Mitchell Stoltz (not reading bugmail)]

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

Comment 55

[Sander]

I mistakenly filed bug 138077 on the following, thinking this patch was already
applied to 0.9.9 Since it wasn't however, I wonder if a correct patch will also
work correctly for this case:

function fatherclose() {
 father = window.self;
 father.opener = window.self;
 father.close();
}

In IE this closes your window without a confirmation dialog. It would be
somewhat good if Mozilla didn't suffer from that.

Comment 56

[Vadim Berezniker]

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

Comment 57

[Jesse Ruderman]

To clarify my question, "Are the pref and dialog necessary?": not having a
dialog or pref would mean automatically denying window.close() in the cases
where IE brings up a dialog.  I think defaulting to "No, you can't close this
window that you didn't open" would be better than bringing up a dialog, in part
because we should avoid adding security dialogs to which the most reasonable
answer on an untrusted/porn site is "yes".  (See bug 68215 comment 10, related
to the IE dialog "Are you sure you want to navigate away from this page?" sites
can create during the onbeforeunload event.)

By the way, I only know of two sites that use window.close() in a way that
causes IE to bring up a dialog.  Both would work fine without the dialog.
http://web.archive.org/web/20010504010346/www.bwgame.com/ (as a joke)
http://camelot.warcry.com/ ("you disable pop-ups? buh bye.")

Comment 58

[Jonas Jørgensen]

Mpt: What's best for the user experience -- confirmation dialog or simply
denying the call to close()?

Comment 59

[Christopher Cook]

Hmm, I don't see Data Loss mentioned in this bug, perhaps because it's a matter
of opinion but if I have 5 tabs open, and one has a form being filled
out/webmail email being written then I WILL lose data because of this bug, not
to mention all of the sites I had open (in my opinion having to re-browse to a
page you found through a series of links is just as bad as having to re-type an
email you've just lost). That said, this bug is getting noisy and I'm just
contributing to that. Regardless of whether a prompt is given to the user a
window should never be able to close tabs/windows it doesn't own (except itself)
so is this getting a patch applied soon? That way we'll have less noise on the
bug even if there is still a question about user prompts. I reckon follow the IE
way and prompt the user if a top level window tries to close itself. Pref it if
you like, but I doubt many people will care and prompting most likely should be
on by default.

Comment 60

[Sander]

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

Comment 61

[Frederic Bezies]

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

Comment 62

[Robert Wall]

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

Comment 63

[Kai Lahmann (is there, where MNG is)]

this seams to be fixed by the fix for 103452. The testcase doesn't work here any
more!

Comment 64

[Jesse Ruderman]

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

Comment 65

[Ray Jefferson]

ckritzer, please retest this bug.  This bug has reported to be fixed, however,
we would like verification in order to close.

Comment 66

[Jerry Baker]

I just came in to Bugzilla to report this very thing. Still able to simply place
a window.close() script in the sole Mozilla window and have it close with no
argument whatsoever. This is using trunk 2002070812.

Comment 67

[Jerry Baker]

Testcase still works here.

Comment 68

[Frederic Bezies]

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

Comment 69

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

It's a big security hole.
I'm not shure if it's in any standards, but every browser use way of disallowing
script, closing windows which wasn't opened by the script.
Mozilla shouldn't allow (without dialog) closing main window (never!), main
frame and so on. Lets say easier. Mozilla should not allow script to close
window opened by human.

Second thing is that Mozilla shouldn't allow closing window, opened from script
in frame if frame location is different host than main window.

Comment 70

[Mitchell Stoltz (not reading bugmail)]

Marking nsbeta1 for re-nomination - I will revive this patch and get it checked in.

Comment 71

[David G King]

As the patch for this bug has an "r" and an "sr", and the tree is not frozen,
then this can be checked in right now.....right?

Comment 72

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

I think it's quite important to fix this bug. Especially if it's ready to push.

Comment 73

[Mats Palmgren (inactive)]

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

Comment 74

[Mitchell Stoltz (not reading bugmail)]

This patch, when I tried it originally, caused some regressions and was backed
out. I'll check it in as soon as I can make sure we've eliminated the regressions.

Comment 75

[Mitchell Stoltz (not reading bugmail)]

Attachment: Simpler patch - no pref, no dialog

One thing to note in this patch is that if the getService fails, or the check
for chrome scripts, we proceed to close the window. While this is slightly less
"safe," I think it's better because in the unlikely occurrence that one of
those calls fails, the close call will proceed and the browser's behavior will
not appear broken. I could go either way on that, though.

Comment 76

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 103376 [details] [diff] [review]
Simpler patch - no pref, no dialog

r=heikki

Your comment that we will close the window if we failed to get script security
manager is not true; we do an early return so we would end up not closing the
window. But I am ok either way, you can decide.

I was wondering for a moment what happens with windows that were opened by
other domains, but of course the normal same origin policy should prevent
scripts from accessing the close() function then.

Comment 77

[Mitchell Stoltz (not reading bugmail)]

Attachment: Minor change - continue on failure

In response to Heikki's comment, I made a minor change so that we really will
continue with the close if we can't ge the security manager (shouldn't ever
happen anyway).

Comment 78

[Johnny Stenback (:jst)]

Comment on attachment 103790 [details] [diff] [review]
Minor change - continue on failure

sr=jst

Comment 79

[/\/\arcio Galli]

All right(TM)!!! ;) great news!!

Comment 80

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 103790 [details] [diff] [review]
Minor change - continue on failure

carrying over Heikki's r=, with his permission.

Comment 81

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 103790 [details] [diff] [review]
Minor change - continue on failure

a=dbaron for trunk checkin.

(Does this allow a script to close a window that was opened by a script on a
different site?  Does that need to be a separate bug?)

Comment 82

[Mitchell Stoltz (not reading bugmail)]

Fixed on trunk.

David: the same-origin policy should prevent a script closing a window it
doesn't own.

Comment 83

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

Attachment: testcase

There are major problems with this patch.
I canot find any rule in this, but few seconds play with this testcase always
end in a crash. Try to create few childs, close them and so on. Very often i
end with a crash, few times i closed one of the 'childs' and all browser was
closed.

Comment 84

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

My build - Mozilla 2002102508 Win2k
Example TB:

TB13084399Y
TB13084376W

Comment 85

[Mitchell Stoltz (not reading bugmail)]

Are you sure those problems are related to this patch? Could they have been
caused by other changes? Do you see the same crashes in yesterday's build?

Comment 86

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

I'll check it.

Link to same testcase - http://alladyn.art.pl/gandalf/MozillaWindowClose/test.html

Comment 87

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

Ok. I launched version from 24th, and same reaction. Sometimes Mozilla closes
all windows instead of one child window, more often crash.
Want more TB?

Comment 88

[Mitchell Stoltz (not reading bugmail)]

aargh...now I remember what the regression with the original patch was -
Tinderbox needs to be able to close windows. I'm reopening this bug to add the
pref back in. I'll try Zbigniew's testcase again too, although I haven't been
able to reproduce his crash yet.

Comment 89

[Barrett Lindstrom]

is this why window.close() no longer works on trunk builds?  I was getting ready
to write up a bug, and came across this one.

Comment 90

[Jerry Baker]

2002110610 trunk closes windows fine as far as I can tell.

Comment 91

[Barrett Lindstrom]

hmmmm..  doesn't work for me.  I'm using this page:
http://blues.mcom.com/users/barrettl/publish/windowclose/restart.html

The http code that closes it is: 
<body
 onload="window.focus(); timeout=setTimeout('window.close()',5000);">

This used to work before builds from 2002-10-25.  Mozilla trunk 2002102415 works
fine, mozilla trunk 2002102508 doesn't.  Is this a new bug?

Comment 92

[Jerry Baker]

That URL appears to be internal. I tested with http://www.bakerweb.biz/faq.html.
Click a link and window opens, clikc "Close Window" and window.close()
successfully closes the window.

Comment 93

[Barrett Lindstrom]

ok, I've posted the test page here:
http://people.netscape.com/barrettl/test/restart.html

The behavior of this page is that it should close the window after a few seconds
automatically.  This works with the branch builds, and trunk builds prior to 10-25

Comment 94

[Jerry Baker]

If I understand correctly, that behavior is correct. The JavaScript does not own
that window, so it should not be able to close it. Try using JS to spawn a
window, and then closing that same window.

Comment 95

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

Yes. URL from comment #93 cannot close window opened by user.

Comment 96

[Barrett Lindstrom]

Ok, I see what's going on now.  Yes, the behavior is correct, but it stops my
tests from working (requires the ability to close windows).  According to
http://bugzilla.mozilla.org/show_bug.cgi?id=32571#c88 there will be a pref added
back in that will fix the problem.  I think this is also related (or the same
thing as), http://bugzilla.mozilla.org/show_bug.cgi?id=177827

Comment 97

[Jerry Baker]

this.close() still works in windows that are not owned.

Comment 98

[Mitchell Stoltz (not reading bugmail)]

Jerry,
   this.close() does not work for me in windows that are not owned. Can you
attach a testcase?

Comment 99

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

I made many tests for this bug and i cannot find any 'security bug' with working
this.close() for windows that are not owned.

Comment 100

[Chris Lyon]

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

Comment 101

[Mitchell Stoltz (not reading bugmail)]

Attachment: New patch with pref and warning message

OK, I've added back the pref, for tinderbox and other testing situations that
need to be able to automate the closing of windows. I also added a localized
message in the JS console window when a close is blocked.

Comment 102

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 108106 [details] [diff] [review]
New patch with pref and warning message

>Index: modules/libpref/src/init/all.js
>===================================================================
>Index: dom/resources/locale/dom.properties
>===================================================================
>Index: dom/src/base/nsGlobalWindow.cpp
>===================================================================
>+        rv = gPrefBranch->GetBoolPref("dom.allow_scripts_to_close_windows",
>+                                &allowClose);

You are not using this rv for anything - either remove or use.

>+          nsCOMPtr<nsIStringBundleService> stringBundleService(
>+            do_GetService(kCStringBundleServiceCID, &rv));

Same here.

Comment 103

[Johnny Stenback (:jst)]

Comment on attachment 108106 [details] [diff] [review]
New patch with pref and warning message

+	 rv = gPrefBranch->GetBoolPref("dom.allow_scripts_to_close_windows",
+				 &allowClose);

Indent &allowClose so that it lines up with the first argument to the method.

sr=jst

Comment 104

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 105

[Michael Lefevre]

now works (or rather is prevented from working) - tested with recent nightly on
win2k.

Comment 106

[M. Creidieki Crouch]

I don't agree with the decision made to not include a dialog box to ask the user
whether to close the window.  The <a
href=http://devedge.netscape.com/library/manuals/2000/javascript/1.3/reference/window.html#1201822>
Client-Side Javascript Reference</a> indicates that a dialog box should be
opened if window.close() attempts to close a window it does not own.

Comment 107

[Jesse Ruderman]

[e-mail]: see bug 190515.