large window.opened window can cover and spoof taskbar if parent is maximized

VERIFIED FIXED in mozilla1.7beta

Status

()

--
major
VERIFIED FIXED
15 years ago
6 years ago

People

(Reporter: jruderman, Assigned: danm.moz)

Tracking

({csectype-spoof, fixed1.4.3})

Trunk
mozilla1.7beta
x86
Windows XP
csectype-spoof, fixed1.4.3
Points:
---
Bug Flags:
blocking1.7a -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix])

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031218
Firebird/0.7+

Steps to reproduce:
1. Maximize your window.
2. javascript:window.open("","","width=,resizable=no")

Result: the new window covers your taskbar

Expected: the new window should not overlap with the taskbar, or at least go
under the taskbar where it overlaps.

This seems to be a z-order problem, not a positioning problem like bug 203621.
(Reporter)

Updated

15 years ago
Whiteboard: security
(Reporter)

Comment 1

15 years ago
I discovered this security hole by pushing the big red button on
http://www.turnofftheinternet.com/.
(Reporter)

Updated

15 years ago
Flags: blocking1.7a?
Created attachment 141284 [details] [diff] [review]
Always size and position maximized windows.

This may be way off target, I don't know my way around window sizing code all
that well, so bear with me here. This attempts to fix this bug by always sizing
a maximized window to its creators size, and that seems to fix this bug.
Attachment #141284 - Flags: review?(danm-moz)

Comment 3

15 years ago
won't hold 1.7a for this since I'm guessing that a very high % of mozilla users
have popup blocking enabled, but we defintely would take a good fix if one
appears in time...
Flags: blocking1.7a? → blocking1.7a-

Comment 4

15 years ago
cc'ing some more folks for reviews and ideas...
(Assignee)

Comment 5

15 years ago
Created attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

The problem is caused by some weird interaction between a maximized but
unmaximizable and unsizable window. With that combination of happenstance, the
system decides on its own to resize the window as it becomes visible.

The window has already been sized (in the chrome docshell's load handler), so
it's not as if it hasn't been given a size. Johnny's patch sets the window size
once again a little later. It pretty much just repeats the previous size,
though slightly larger. Both size commands happen before the window is made
visible (and resized by the system), so it seems the second attempt just
happens at a time more propitious for convincing the system to leave it alone.

Well if it works, be happy. That said, I think I'd prefer this patch, which
prevents a new window which can't be resized (or maximized) by the user from
being created in a maximized state in the first place.* I claim this will also
fix the bug, and seems more right to me. (I can't test or even compile the
patch as posted without a lot of trouble, but I did test a similar version that
works in my heavily modified tree.)

* Note the new window is still given the size of its maximized opener. I think
this is also a bug, but outside the scope of this bug (see bug 231843).
Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

This works, I like it much better than my attempt at this! sr=jst
Attachment #141354 - Flags: superreview+
(Assignee)

Updated

15 years ago
Attachment #141354 - Flags: review?(bryner)
Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

- Why did you need to static_cast to nsIXULWindow?

- You might want to just add a short comment to the code about what this is
preventing.

Fix those (or not, if the cast is really necessary, but I'd like to know why)
and r=me.
Attachment #141354 - Flags: review?(bryner) → review+
(Assignee)

Comment 8

15 years ago
It's standard multiple inheritance ambiguous root class stuff. do_GetInterface
wants an nsISupports and it has four to choose from in class nsXULWindow. I
suppose this would be more correct (though identical in generated code):

+ nsCOMPtr<nsIWebBrowserChrome> chrome(do_GetInterface(
+                                      NS_ISUPPORTS_CAST(nsIXULWindow *, this)));

though this file is already full of the NS_STATIC_CAST kind used in this patch.
(Assignee)

Updated

15 years ago
Assignee: general → danm-moz
Target Milestone: --- → mozilla1.7beta
(Assignee)

Comment 9

15 years ago
Fix checked in to the trunk for Mozilla 1.7b / Firefox 0.9.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

15 years ago
Took me a while to find the checkin.. was actually on Feb 21.
(Reporter)

Comment 11

15 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b) Gecko/20040224
Firefox/0.8.0+

Firebird is fixed too :)  I guess that file isn't forked.

Updated

15 years ago
Attachment #141284 - Flags: review?(danm-moz)
verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7b)
Gecko/20040421
Status: RESOLVED → VERIFIED
Adding Jon Granrose to CC list to help round up QA resources for verification
Comment on attachment 141354 [details] [diff] [review]
unsizable windows are no longer born maximized

a=blizzard
Attachment #141354 - Flags: approval1.4.3+
Checked in to the 1.4 branch.
Keywords: fixed1.4.3
Whiteboard: security → [sg:fix]security
Group: security
(Reporter)

Updated

6 years ago
Keywords: csec-spoof
Whiteboard: [sg:fix]security → [sg:fix]
You need to log in before you can comment on or make changes to this bug.