Closed Bug 32571 Opened 20 years ago Closed 17 years ago

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

Categories

(Core :: Security, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla1.2alpha

People

(Reporter: gaxzero, Assigned: security-bugs)

References

()

Details

(Keywords: dataloss, Whiteboard: [ADT2])

Attachments

(3 files, 3 obsolete files)

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
*** Bug 32580 has been marked as a duplicate of this bug. ***
*** Bug 32580 has been marked as a duplicate of this bug. ***
Maybe DOM0, but seems like a security issue.
Assignee: rogerl → norris
Component: Javascript Engine → Security: General
QA Contact: rginda → junruh
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Target Milestone: --- → M17
Bulk reassigning most of norris's bugs to mstoltz.
Assignee: norris → mstoltz
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Target Milestone: M17 → M20
This is worthwhile looking into. JS should NOT be able to close windows it
doesn't own. Marking M20 - to be looked at later.
Assigning QA to czhang
QA Contact: junruh → czhang
Blocks: 48983
This is kinda bad, allows a script to quit the browser. Not damaging but
annoying. Recommend nsbeta3 but setting to P4, lower priority.
Keywords: nsbeta3
Priority: P3 → P4
Putting on [nsbeta3-] per PDT review today.  Other more critical work to do for 
nsbeta3 and rtm
Whiteboard: [MINUS][nsbeta3-]
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. 
Keywords: 4xp
Whiteboard: [MINUS][nsbeta3-] → [nsbeta3-]
Future.
Target Milestone: M20 → Future
Future?
I think this is potentialy really bad....
Summary: window.close() closes windows it doesn't own → close() can close windows it doesn't own
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?
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....!"
QA Contact: czhang → junruh
Mass changing QA to ckritzer.
QA Contact: junruh → ckritzer
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.
Depends on: 36050
*** Bug 66797 has been marked as a duplicate of this bug. ***
*** Bug 74791 has been marked as a duplicate of this bug. ***
Hardware: PC → All
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.)
*** Bug 67700 has been marked as a duplicate of this bug. ***
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).
Target is now 0.9.4, Priority P2.
Priority: P4 → P2
Target Milestone: Future → mozilla0.9.4
Keywords: nsbeta3
Whiteboard: [nsbeta3-]
Target Milestone: mozilla0.9.4 → mozilla0.9.5
*** Bug 96952 has been marked as a duplicate of this bug. ***
time marches on...retargeting to 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
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).
Whiteboard: mozilla0.9.6
*** Bug 104889 has been marked as a duplicate of this bug. ***
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");
Moving the most time-critical bugs and minor security fixes to 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
*** Bug 111187 has been marked as a duplicate of this bug. ***
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
Target Milestone: mozilla0.9.7 → mozilla0.9.8
*** Bug 115823 has been marked as a duplicate of this bug. ***
*** Bug 115959 has been marked as a duplicate of this bug. ***
It missed the 0.9.8 train. However nominating for 1.0 to not forget this bug.
Keywords: mozilla1.0
Whiteboard: mozilla0.9.6
0.9.9
Target Milestone: mozilla0.9.8 → mozilla0.9.9
*** Bug 122999 has been marked as a duplicate of this bug. ***
*** Bug 119419 has been marked as a duplicate of this bug. ***
Attachment #69560 - Flags: superreview+
Comment on attachment 69560 [details] [diff] [review]
Patch - add pref and confirmation dialog

sr=jst
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.
Attachment #69560 - Flags: review+
mstoltz patch backed out.
*** Bug 97015 has been marked as a duplicate of this bug. ***
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). 
Keywords: nsbeta1
a=asa (on behalf of drivers) for checkin to 0.9.9
Keywords: mozilla0.9.9
looks like partial checkins were backed out. remaining checkins caused 
regression bug 127938
Are the pref and dialog necessary?  I don't see a need for either.
Let's try to finish this for mozilla1.0
Target Milestone: mozilla0.9.9 → mozilla1.0
adding topembed, we don't want to have Gecko powered products closing the
document window easily. :)
Keywords: topembed
ADT2 per ADT triage.
Whiteboard: [ADT2]
EDT plussing - future embedding clients should have this.
Keywords: topembedtopembed+
*** Bug 132924 has been marked as a duplicate of this bug. ***
*** Bug 133904 has been marked as a duplicate of this bug. ***
*** Bug 134825 has been marked as a duplicate of this bug. ***
I think this bug is as important as Bug 103452
*** Bug 136081 has been marked as a duplicate of this bug. ***
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.
*** Bug 139177 has been marked as a duplicate of this bug. ***
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.")
Depends on: 103452
Mpt: What's best for the user experience -- confirmation dialog or simply
denying the call to close()?
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.
*** Bug 149105 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.0 → mozilla1.2alpha
*** Bug 150169 has been marked as a duplicate of this bug. ***
*** Bug 151123 has been marked as a duplicate of this bug. ***
this seams to be fixed by the fix for 103452. The testcase doesn't work here any
more!
*** Bug 151469 has been marked as a duplicate of this bug. ***
Summary: close() can close windows it doesn't own → window.close() can close windows it doesn't own
ckritzer, please retest this bug.  This bug has reported to be fixed, however,
we would like verification in order to close.
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.
Testcase still works here.
*** Bug 158073 has been marked as a duplicate of this bug. ***
Blocks: 158464
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.
Marking nsbeta1 for re-nomination - I will revive this patch and get it checked in.
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?
Blocks: 1.2a
Blocks: 1.2b
No longer blocks: 1.2a
I think it's quite important to fix this bug. Especially if it's ready to push.
*** Bug 170165 has been marked as a duplicate of this bug. ***
Keywords: dataloss
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.
Blocks: 1.2
No longer blocks: 1.2b
Keywords: 4xp
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 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.
Attachment #103376 - Flags: review+
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).
Attachment #103376 - Attachment is obsolete: true
Comment on attachment 103790 [details] [diff] [review]
Minor change - continue on failure

sr=jst
Attachment #103790 - Flags: superreview+
All right(TM)!!! ;) great news!!
Comment on attachment 103790 [details] [diff] [review]
Minor change - continue on failure

carrying over Heikki's r=, with his permission.
Attachment #103790 - Flags: review+
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?)
Attachment #103790 - Flags: approval+
Fixed on trunk.

David: the same-origin policy should prevent a script closing a window it
doesn't own.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Attached file 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.
My build - Mozilla 2002102508 Win2k
Example TB:

TB13084399Y
TB13084376W
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?
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?
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
2002110610 trunk closes windows fine as far as I can tell.
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?
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.
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
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.
Yes. URL from comment #93 cannot close window opened by user.
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
this.close() still works in windows that are not owned.
Jerry,
   this.close() does not work for me in windows that are not owned. Can you
attach a testcase?
I made many tests for this bug and i cannot find any 'security bug' with working
this.close() for windows that are not owned.
No longer blocks: 1.2
*** Bug 175857 has been marked as a duplicate of this bug. ***
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.
Attachment #69560 - Attachment is obsolete: true
Attachment #103790 - Attachment is obsolete: true
Attachment #108106 - Flags: superreview?(jst)
Attachment #108106 - Flags: review?(heikki)
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.
Attachment #108106 - Flags: review?(heikki) → review+
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
Attachment #108106 - Flags: superreview?(jst) → superreview+
Fix checked in.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
QA Contact: ckritzer → bsharma
now works (or rather is prevented from working) - tested with recent nightly on
win2k.
Status: RESOLVED → VERIFIED
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.
No longer blocks: 158464
Blocks: 158464
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.