Dependency of JavaScript objects is Misconfigured Browser Crashes.

Bug #1638610 reported by Dhiraj on 2016-11-02
256
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Confirmed
Medium
firefox (Ubuntu)
Undecided
Unassigned

Bug Description

Hey Team ,

The bug i want to mention here is a denial of service attack that will not allow any kind of redirection on a page crafted by attacker where we have used hyper-links(ahref).
The bug can be maliciously used by crafting an HTML file by an attacker and then sending it to the victim clearly showing there is a hyper-link that redirects to lets say (google.com) through status bar but it will not , instead cause denial of service , browser's also hang up and Crashes.
I have tested it on the Very Latest Version of Ubuntu LTS Default Browser.

Reason:
The following script stops the page from being redirected:
window.onbeforeunload = function(){
//Unredirectable Page
setTimeout("window.location=document.location;",0);
}

Demo URL : http://hackies.in/Unredirect-Browsers-Test.html

Actual results:

It should redirect me to the new page , where as it don't redirect to a new page and the browsers Hangs up.

Expected results:

So dependency of JavaScript objects(window.document) on Href attribute should not be there.
Attached POC for References

Revision history for this message
In , Smylers-u (smylers-u) wrote :

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160405015839

Steps to reproduce:

When cautiously investigating a suspicious looking email, I tried to report a page using ‘Report deceptive site...’, but doing so just redirected me to another spam webpage instead of the report page.

An email tricked me into opening it by claiming to be from well-known shop Marks & Spencer, but on reading it clearly wasn't. It linked to this page, which I cautiously opened in a private window to find out what it was doing and whether the site should be reported: n.mobzones.com/uk/m-s?offer=271

I clicked on the answer ‘Marks & Spencer’, which went to a page with a really long URL (but still http:, not data: or anything funky).

Then I pressed Alt+H and chose ‘Report deceptive site...’.

Actual results:

The question popped up asking if I was sure I wanted to leave the page.

I chose ‘Leave Page’, and was redirected (via a bunch of ad networks) to a dating website.

I went back, repeated, chose ‘Stay on Page’, and was still redirected to the same dating site.

Expected results:

I should've reached the site for reporting the page I was on.

Ideally ‘Report deceptive site...’ wouldn't ever trigger the confirmation for leaving the page, but if it does then ‘Leave Page’ should go to the reporting site and ‘Stay on Page’ should stay there; neither of them should redirect to a completely different page.

(Please note I'm not saying this specific site definitely merits being reported. Merely the form for doing so should have been displayed.)

Revision history for this message
In , Dolske (dolske) wrote :

Huh, yeah. When I load http://n.mobzones.com/uk/m-s?offer=271, attempting to report the site does exactly what you describe. This isn't actually a problem specifically to Report a Site, entering "google.com" in the URL bar also results in the page somehow canceling that load and you end up on another spam page. (http://play.leadzupc.com/...).

This seems pretty annoying, sites shouldn't be able to trap the user like this.

Gijs/mconley: perhaps one of you could take a look to see what's happening?

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

(In reply to Justin Dolske [:Dolske] from comment #1)
> Huh, yeah. When I load http://n.mobzones.com/uk/m-s?offer=271, attempting to
> report the site does exactly what you describe. This isn't actually a
> problem specifically to Report a Site, entering "google.com" in the URL bar
> also results in the page somehow canceling that load and you end up on
> another spam page. (http://play.leadzupc.com/...).
>
> This seems pretty annoying, sites shouldn't be able to trap the user like
> this.
>
> Gijs/mconley: perhaps one of you could take a look to see what's happening?

This page has:

        $(window).on("beforeunload", function (e) {
            $(window).unbind("beforeunload");

            setTimeout(function () {
                window.location = url;
            }, 0);

            e.returnValue = message;
            return message;
        });

Boris, are there spec reasons we can't just stop navigation permanently as soon as we fire beforeunload, until either the page has been unloaded / destroyed, or the user explicitly decides to stay on the page?

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

> are there spec reasons we can't just stop navigation permanently as soon as we fire beforeunload

At first blush, per spec you can't stop navigation inside beforeunload at all. Or something. Figuring out what the spec says here is rocket science. :(

Revision history for this message
In , Overholt-u (overholt-u) wrote :

Anne, can you help (see comments 2 and 3).

Revision history for this message
In , Annevk (annevk) wrote :

First of all, agreed with bz that this is in need of heavy refactoring. Unfortunately there has been a lot of that, so I haven't gotten around to it yet. If we had more editors, maybe...

Having read through navigate and its various associated algorithms a few times now, I don't think anything there stops this, since the user is allowed to navigate elsewhere until the page navigated to actually starts arriving (at that point navigate starts blocking certain navigation attempts, though not all).

Now, we could make tweaks, but that's tricky. E.g., if the user clicks <a onclick=window.location='/x'>test</a> after dismissing the dialog, should that fail or work?

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

(In reply to Anne (:annevk) from comment #5)
> Now, we could make tweaks, but that's tricky. E.g., if the user clicks <a
> onclick=window.location='/x'>test</a> after dismissing the dialog, should
> that fail or work?

Can we use popup-blocker-style "allowed from user interaction, not allowed otherwise" logic or is that problematic?

Revision history for this message
In , Annevk (annevk) wrote :

That might work, though I'm not looking forward to adding that to something that is already impossible to understand. I guess you'd have some document-wide-flag set during "prompting to unload" (beforeunload). And we'd check that flag in navigation and if it's set also require the popup check to pass. Hmm.

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

Or we could have a "in unload prompt" flag that gets propagated to timeouts set while that flag is set and any code those timeouts run etc.

Revision history for this message
In , Annevk (annevk) wrote :

That might be hard given the numerous other APIs we have for queuing tasks and task-like things, no? E.g., setImmediate() if that is still going to be a thing, or requestAnimationFrame().

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

Yes, I'm not saying it would be simple or easy. :(

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

(In reply to Boris Zbarsky [:bz] from comment #8)
> Or we could have a "in unload prompt" flag that gets propagated to timeouts
> set while that flag is set and any code those timeouts run etc.

var gotbeforeunload = false;
setInterval(function() {
 if (gotbeforeunload) {
   location.href = "foo";
 }
}, 100);
onbeforeunload = function() { gotbeforeunload = true; };

would defeat that, AIUI. :-(

(and I've seen patterns like this in use on trappy sites before :-( )

Really, the whole beforeunload thing needs to go away, but then there's the rest of the web to think of. :-(

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

I almost wonder if we should restrict beforeunload to https and/or a permission...

Revision history for this message
In , Overholt-u (overholt-u) wrote :

It doesn't sound like we're going to be able to fix this anytime soon :(

Revision history for this message
In , Overholt-u (overholt-u) wrote :

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

Revision history for this message
In , Arni2033 (arni2033) wrote :

Just in case, this page prevents any navigation (including Back/Forward buttons):
> data:text/html,<body onbeforeunload="onbeforeunload=null;setTimeout('location.reload()',0)">

Also, bug 1283493 is probably a duplicate.

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
Dhiraj (mishra-dhiraj95) wrote :
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Can you please report this issue to upstream Firefox developers and then link the bug reports?

Thanks

information type: Private Security → Public Security
Revision history for this message
Dhiraj (mishra-dhiraj95) wrote :

Hi ,

Please find the Link below :

https://bugzilla.mozilla.org/show_bug.cgi?id=1313918

Thanks

Changed in firefox (Ubuntu):
status: New → Confirmed
Changed in firefox:
importance: Unknown → Medium
status: Unknown → Confirmed
Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

My recollection is that the current version of the spec tries to distinguish user-initiated navigation and, once we start trying to execute that (by firing beforeunload, followed by unload, followed by actually loading a new page), any subsequent manipulation of location should have no effect (unless the user bails out in beforeunload). (Paraphrasing spec language for something roughly like this, obviously)

Boris/Anne, is that recollection at all near the truth? What should we be doing here? And any chance we can re-prio this?

Revision history for this message
In , Annevk (annevk) wrote :

No, the situation around this specification-wise hasn't really improved from three years ago unfortunately.

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #3)
> > are there spec reasons we can't just stop navigation permanently as soon as we fire beforeunload
>
> At first blush, per spec you can't stop navigation inside beforeunload at
> all. Or something. Figuring out what the spec says here is rocket science.
> :(

(In reply to Anne (:annevk) from comment #5)
> First of all, agreed with bz that this is in need of heavy refactoring.
> Unfortunately there has been a lot of that, so I haven't gotten around to it
> yet. If we had more editors, maybe...
>
> Having read through navigate and its various associated algorithms a few
> times now, I don't think anything there stops this, since the user is
> allowed to navigate elsewhere until the page navigated to actually starts
> arriving (at that point navigate starts blocking certain navigation
> attempts, though not all).
>
> Now, we could make tweaks, but that's tricky. E.g., if the user clicks <a
> onclick=window.location='/x'>test</a> after dismissing the dialog, should
> that fail or work?

(In reply to Anne (:annevk) from comment #22)

> No, the situation around this specification-wise hasn't really improved from three years ago unfortunately.

So... I'm going to be quite tentative here, because I'm very much not an expert, and both of you are. But if I'm reading:

https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate

Specifically,

> If there is a preexisting attempt to navigate browsingContext, and the source browsing context is the same as browsingContext, and that attempt is currently running the unload a document algorithm, then return without affecting the preexisting attempt to navigate browsingContext.

> If the prompt to unload algorithm is being run for the active document of browsingContext, then return without affecting the prompt to unload algorithm.

I assume that the second of these now does mean that we can avoid navigation from within beforeunload itself, right?

I'm guessing (it's a bit hard to tell for me, not knowing where to look in the spec) that we don't unload the current document (ie we don't hit the first cited condition) until we start getting a response for the initial navigation, and so between these two conditions (ie after `beforeunload` and before `unload`), timers (setTimeout etc.) and any other events may trigger JS that starts another navigation, and per spec that should override the earlier navigation.

As Anne points out at the end of comment 5, blocking all navigation after 1 navigation has started is tricky.

I'm tempted to suggest we try to pass whether a navigation was the result of a user action (similar/using popup blocking state) down to loadinfo in some way, and make user-triggered navigation be un-overridable by webpage navigations. That would, AIUI fix this bug (and all its variations).

It's also a bit complex, and I would be happy to hear other alternatives - in particular, I'm quite curious if any of these bugs affect other browsers and what, if any, mitigations they have in place.

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

> I assume that the second of these now does mean that we can avoid navigation from within beforeunload itself, right?

Yes, and we do. See nsDocShell::IsNavigationAllowed.

> we don't unload the current document (ie we don't hit the first cited condition) until we start getting a response for the initial navigation

That's correct. It has to be that way, because the response could be a type that we'd hand off to a helper app instead of handling internally, so we can't know whether we'll be unloading at all until we get the response headers.

I too would be interested in what other browsers do. Conceptually, treating "user-triggered" and "page just randomly did it" navigations differently makes sense to me.

Revision history for this message
In , Gijskruitbosch+bugs (gijskruitbosch+bugs) wrote :

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

Revision history for this message
In , Pbz-a (pbz-a) wrote :

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

Revision history for this message
In , Gingerbread-man-2 (gingerbread-man-2) wrote :

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

Revision history for this message
In , Zitrobugs (zitrobugs) wrote :

1.Step: open https://mozhelp.dynvpn.de/dateien/wl
2.Step: click on a bookmark in Firefox. For example http://example.org/foo (not a mozilla.org bookmark)
Expected result: it should open your bookmark in the same tab.

In Google Chrome/Opera it opens the wanted bookmark when I click on it.
In IE 11 and Edge and Firefox i can not override the actual tab with my bookmark

Revision history for this message
In , Dlee-t (dlee-t) wrote :

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

To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.