The default bug view has changed. See this FAQ.

"open a blocked popup" feature uses the global window as the opener for blocked popups (variant of bug 343175)

RESOLVED FIXED

Status

SeaMonkey
General
--
major
RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: Gavin, Assigned: Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com])

Tracking

({fixed-seamonkey1.0.8, fixed-seamonkey1.1.1})

1.8 Branch
fixed-seamonkey1.0.8, fixed-seamonkey1.1.1
Bug Flags:
blocking-seamonkey1.0.8 +
blocking-seamonkey1.1 -
blocking-seamonkey1.1.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate?], URL)

Attachments

(3 attachments, 3 obsolete attachments)

SeaMonkey is still vulnerable to the XSS exploit reported in bug 343175, because it always uses the topmost window as the opener for blocked popups, instead of the window that originally tried to open the popup.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/browser/resources/content/navigator.js&rev=1.596&mark=2442#2439

The testcase in bug 343175 does not work, since the frame injection bug that it uses (bug 343168) was also fixed. I will attach a new testcase.

I tested the 1.0.5 candidate build, 1.8 branch and trunk builds should be equally affected.
Created attachment 238153 [details]
attacker
Created attachment 238154 [details]
victim
To test this bug, load https://recluse.mozilla.org/attachment.cgi?id=238154 .

Expected results: "My opener was bugzilla.mozilla.org"
Actual results: "My opener was recluse.mozilla.org"
I'd ask for blocking-seamonkey1.0.6 if that flag existed - I don't really think this is critical enough to block the 1.0.5 release at this stage.

Comment 5

11 years ago
jst, any idea why nsGlobalWindow::FireAbuseEvents doesn't fire the events on the document of the window that tried to open the blocked popup? Instead the UI code has to grovel through the frameset looking for a window that it thinks matches..
FireAbuseEvents fires on the window on which open() was called...  Generally, detecting which window _originated_ the load is Hard.
Flags: blocking-seamonkey1.5a?
Flags: blocking-seamonkey1.1b?
Flags: blocking-seamonkey1.0.6?
This should be easier to fix once bug 354973 is fixed.
Flags: blocking-seamonkey1.0.6? → blocking-seamonkey1.0.7?
Were the blocking-seamonkey-*? flags on this bug ignored because the people driving SeaMonkey releases can't see this bug?
Flags: blocking-seamonkey1.1b? → blocking-seamonkey1.1?
Neil and biesi can see these bugs.
http://wiki.mozilla.org/SeaMonkey:Project_Organization
I know that Neil and biesi are in the security group, but I made the (perhaps incorrect) assumption based on bugmail that KaiRo and CTho are the ones that deal with most SeaMonkey blocking requests, so I thought that might explain why this bug's blocking nominations weren't dealt with. I see now that they were made about 2 days before the associated releases, though, so I guess it probably wouldn't have mattered either way.
Created attachment 248805 [details] [diff] [review]
patch

dveditz, neil, I don't really care who reviews and who superreviews.  neil, I'm not sure if dveditz actually wants to review this.
Assignee: general → cst
Status: NEW → ASSIGNED
Attachment #248805 - Flags: superreview?(neil)
Attachment #248805 - Flags: review?(dveditz)
Comment on attachment 248805 [details] [diff] [review]
patch

It's important to have prefs set the right way when testing patches :(
Attachment #248805 - Attachment is obsolete: true
Attachment #248805 - Flags: superreview?(neil)
Attachment #248805 - Flags: review?(dveditz)
Created attachment 248808 [details] [diff] [review]
patch v2

This depends on attachment 245720 [details] [diff] [review].  Unfortunately I can't test it, as that patch is in some garbage format that patch doesn't understand, and it's way too much to hack in by hand.
Attachment #248808 - Flags: superreview?(neil)
Attachment #248808 - Flags: review?(dveditz)

Comment 14

10 years ago
(In reply to comment #8)
>Were the blocking-seamonkey-*? flags on this bug ignored because the people
>driving SeaMonkey releases can't see this bug?
Personally I was waiting on the fix to what turned out to be bug 354973.
(In reply to comment #14)
> Personally I was waiting on the fix to what turned out to be bug 354973.

That's fine, I'm just asking because the blocking flag was neither denied nor granted, which made it seem as though it was being ignored. When I made that comment, I didn't realize the blocking request was made so close to the release.

Comment 16

10 years ago
Is it correct that this is not fixed in Firefox 1.5.0.9 and 2.0.0.1 yet? If so, we can probably ship the already tagged SeaMonkkey 1.0.7 unfixed and include a fix in 1.1 and 1.0.8
(In reply to comment #16)
> Is it correct that this is not fixed in Firefox 1.5.0.9 and 2.0.0.1 yet?

Yes, that's correct. If I knew that the next releases were this close I probably wouldn't have asked for blocking.

Comment 18

10 years ago
Comment on attachment 248808 [details] [diff] [review]
patch v2

Wrong sort of popup event!
You need to save the name etc. from onPopupBlocked() - it might be simpler to save the whole event (ask jst if that's reasonable).
Attachment #248808 - Flags: superreview?(neil) → superreview-

Comment 19

10 years ago
As Firefox does not ship a fix yet, this should be pushed out to the next releases, but it really blocks those.
Flags: blocking-seamonkey1.1?
Flags: blocking-seamonkey1.1.1+
Flags: blocking-seamonkey1.1-
Flags: blocking-seamonkey1.0.8+
Flags: blocking-seamonkey1.0.7?
Created attachment 249816 [details] [diff] [review]
patch v3

Neil, I'm not sure what erroneous code you wanted me to remove.  Sorry.  I cleaned up various whitespace stuff in nearby code.
Attachment #248808 - Attachment is obsolete: true
Attachment #249816 - Flags: superreview?(neil)
Attachment #249816 - Flags: review?(neil)
Attachment #248808 - Flags: review?(dveditz)

Comment 21

10 years ago
Comment on attachment 249816 [details] [diff] [review]
patch v3

>-    separator.hidden = !createShowPopupsMenu(event.target);
>+    separator.hidden = !createShowPopupsMenu(event);
> }
> 
>-function createShowPopupsMenu(parent) {
>+function createShowPopupsMenu(event)
>+{
>+  var parent = event.target;
This was the spurious change that you don't need.

>   while (parent.lastChild && parent.lastChild.hasAttribute("uri"))
>     parent.removeChild(parent.lastChild);
Now that you no longer set the uri attribute you need another way to detect your dynamically added menuitems (none of which have ids for instance).

>-  if  (popupUrls.length == 0)
>+  if (popups.length == 0)
This was the whitespace change that I was looking for :-)

>+  var popup = target.popup;
popup doesn't always exist (see above).

>+  var reqWin = popup.reqWin;
>+  var uri = popup.url.spec;
>+  if (reqWin && reqWin.document == popup.reqDoc && uri) {
reqWin and uri will always exist if popup does.

>+    reqWin.open(uri, popup.name, popup.features); /// trunk-only???? (for name... s/popup.name/""/ ?)
/// ????
Attachment #249816 - Flags: superreview?(neil)
Attachment #249816 - Flags: superreview+
Attachment #249816 - Flags: review?(neil)
Attachment #249816 - Flags: review-
Created attachment 250015 [details] [diff] [review]
v4
Attachment #249816 - Attachment is obsolete: true
Attachment #250015 - Flags: review?(neil)

Updated

10 years ago
Attachment #250015 - Flags: review?(neil) → review+
Depends on: 354973
Fixed on trunk
Assignee: cst → general
Status: ASSIGNED → NEW
Attachment #250015 - Flags: approval-seamonkey1.1.1?
Attachment #250015 - Flags: approval-seamonkey1.0.8?
Assignee: general → cst
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 24

10 years ago
Comment on attachment 250015 [details] [diff] [review]
v4

a=me for 1.0.8 and 1.1.1
Attachment #250015 - Flags: approval-seamonkey1.1.1?
Attachment #250015 - Flags: approval-seamonkey1.1.1+
Attachment #250015 - Flags: approval-seamonkey1.0.8?
Attachment #250015 - Flags: approval-seamonkey1.0.8+

Updated

10 years ago
Flags: blocking-seamonkey1.5a?
Whiteboard: [sg:moderate?]
Landed on both branches.
Keywords: fixed-seamonkey1.0.8, fixed-seamonkey1.1.1

Comment 26

10 years ago
This broke popup unblocking on the 1.8.0 branch.  There are no menu items for unblocking and the context menu for the blocker icon is just a tiny square.
Is there a bug on that?  That should block upcoming releases...
I may have merged incorrectly... 1.8.0 differed from trunk much more than 1.8 did.  I'll look into it in a few minutes.
-    var popup = popups[i];
+    var popup = browser.popups[i];

<CTho> NeilAway: do i need r/a, or just land it as bustage fix?
<NeilAway> CTho: just land it

Fixed.
Group: security

Updated

6 years ago
Duplicate of this bug: 370341
You need to log in before you can comment on or make changes to this bug.