showModalDialog doesn't show a location bar, making spoofing trivial

RESOLVED FIXED in mozilla1.9alpha8

Status

()

P1
normal
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: jruderman, Assigned: jst)

Tracking

({csectype-spoof, sec-moderate, testcase})

Trunk
mozilla1.9alpha8
csectype-spoof, sec-moderate, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
wanted1.8.1.x -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
Created attachment 278445 [details]
testcase

If you click the "open a modal dialog" button in the testcase, you'll see that it opens a sheet with the same size as the browser window, perfectly covering all the toolbars.  This would make it trivial for a malicious site to put up a fake address bar making it look like you're on a different site.  The only hints are the lack of a resizing grippy (lower right of the window) and the "sheet coming down" animation.

showModalDialog() sheets probably need the same treatment that window.open() got in bug 337344 to always show some kind of location bar.
Flags: blocking1.9?
Jesse:  Firefox 2 on Windows doesn't even open a modal dialog, saying showModalDialog doesn't exist.  Regression of some kind?

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/2007072518 Firefox/2.0.0.6
(Reporter)

Comment 2

11 years ago
showModalDialog was added after Firefox 2 (in bug 194404).
Assignee: nobody → jst
Flags: blocking1.9? → blocking1.9+
(Assignee)

Comment 3

11 years ago
Created attachment 279995 [details] [diff] [review]
Make modal dialogs obey window feature prefs.

This fixes the problem with modal dialogs not obeying the prefs that control what window features are forced on windows created by scripts, and this makes us match IE closer too. The problem was that due to the code path we end up going through when opening up a modal dialog we end up pushing null on the context stack before entering the window watcher, which makes it think chrome is doing the opening and ignores the prefs in question. That's obviously bad. And in addition to that, I had to tweak a macro that also made us ignore the prefs for dialogs, and modal content dialogs are treated as dialogs (aDialog is true) in the window watcher code.
Attachment #279995 - Flags: superreview?(jonas)
Attachment #279995 - Flags: review?(jonas)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9 M8
Attachment #279995 - Flags: superreview?(jonas)
Attachment #279995 - Flags: superreview+
Attachment #279995 - Flags: review?(jonas)
Attachment #279995 - Flags: review+
(Assignee)

Updated

11 years ago
Attachment #279995 - Flags: approval1.9?

Updated

11 years ago
Attachment #279995 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 4

11 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
I viewed the testcase, and then used the History menu to get back to the bug. Now I'm in a window with no toolbars or location bar. Doh.

Seems like history shouldn't apply to dialogs; is this made moot by this patch, or is there another bug here?
(Assignee)

Comment 6

11 years ago
Um, I have no idea what went wrong there. Doesn't sound related to what went in to fix this in any case. If I go to the testcase, open a modal dialog, there's no history there (as nothing else has loaded in that window yet), and if I close the dialog and go back to the bug in the original window things work as expected here. Can you reliably reproduce what you're seeing?
I filed bug 395465 for the issue I'm seeing on OS X.
(Reporter)

Updated

11 years ago
Whiteboard: [sg:moderate spoof]
So...  this regressed bug 294440.  That macro wasn't checking the chrome thing for a reason.

Updated

11 years ago
Depends on: 402866
Flags: wanted1.8.1.x-
Whiteboard: [sg:moderate spoof] → [sg:moderate spoof] post 1.8-branch
(Reporter)

Updated

5 years ago
Keywords: csec-spoof, sec-moderate
Whiteboard: [sg:moderate spoof] post 1.8-branch
You need to log in before you can comment on or make changes to this bug.