602286 - WebSockets onClose allows for redirect to another site upon leaving the page

Status: RESOLVED WONTFIX

(Core :: Networking: WebSockets, defect, P3)

Description

[Ivo Wetzel]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20101006 Firefox/4.0b7pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20101006 Firefox/4.0b7pre

It turns out that you can do some trickery with the onClose event of a WebSocket, it seems that the order in which this is handled in comparison to window.onload etc. is broken.

Reproducible: Always

Steps to Reproduce:
1. Establish a web socket connection
2. In the onclose handler change do document.location.href = 'http://yahoo.com'
3. Click any link on the page or enter something in the address bar and hit enter
4. Search happily with Yahoo!
Actual Results:  
User is being redirect to yahoo.com upon leaving the page.

Expected Results:  
Any actions in the onclose event that affect the location should be ignored when leaving the apge.

Comment 1

[Ivo Wetzel]

Attachment: Redirect demo

Basic demo of the bug in action. Click the google link, you'll end up at yahoo.

Comment 2

[Ivo Wetzel]

Attachment: Better demo

Apparently clicking the link doesn't work 100% of the time, while using the url bar does. Seems to be a race condition, anyways, adding more socket connections makes clicking the link work reliable.

Comment 3

[Johnny Stenback (:jst)]

Honza, care to look into this? We prevent loading other urls from onunload, we should do the same here.

Comment 4

[Honza Bambas (:mayhemer)]

Depends on resolution of bug 616733 if this should block or not.  removing the flag for now.

Comment 5

[Jason Duell]

Honza,

Since WS is re-enabled now, we need to fix this.  I can take it if you're too busy.  Let me know.

Comment 6

[Honza Bambas (:mayhemer)]

(In reply to Jason Duell (:jduell) from comment #5)
> Honza,
> 
> Since WS is re-enabled now, we need to fix this.  I can take it if you're
> too busy.  Let me know.

Feel free to take this bug.

Comment 7

[Jason Duell]

Brian,

Do you have any sense of what sort of security level (if any) this bug deserves?  I say "if any" because WS code is generally loaded from the server that served the page, and even when it isn't, it's loaded from a domain determined by the page-server.  So I'm not clear on how the evil use-case is for a WS redirecting during onclose.

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I am less familiar with the user-facing security aspects of the browser, so I will defer to Jesse and Dan.

Comment 9

[Jason Duell]

Jesse/Dan:

Any sense of how urgent/serious this is?

I don't know the mechanism for how we've blocked this for XHR, so pointers to a bug would be handy if you've got them (otherwise I'll look into this when I've got 'more important' bugs in websockets--ie when I'm done bringing our protocol up to the latest version of the spec)

Comment 10

[Jesse Ruderman]

Not urgent. I'm pretty sure there are other ways to keep from being navigated away from (see the "eviltraps" metabug, bug 432687).

Comment 11

[Josh Matthews [:jdm]]

I spent an afternoon looking into this, as well as the mechanism by which we prevent changes in unload handlers. The latter works by performing checks to see if the page should allow navigation in every path in which that can occur, however when the websocket onclose handler runs (in a long-running connection that is torn down during page unloading) the page has not actually started unloading yet, so the navigation proceeds merrily on its way.

Comment 12

[Honza Bambas (:mayhemer)]

Releasing, any one feel free to take it.

Comment 13

[Jason Duell]

The impression I've got from bz and jruderman is that this is not a high-priority issue (there's lots of other open 'eviltraps'-see comment 10), and I got no path forward for fixing it, so I'm not going to work on it any time soon.  

I'm happy to mark WONTFIX, but got the sense that we want to eventually work on eviltraps?

Comment 14

[Jesse Ruderman]

Hopefully this will be fixed by the same mechanism as bug 391834.

Comment 15

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 16

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 17

[Daniel Veditz [:dveditz]]

Ultimately, how is this different than having an onclick() handler on the link? A click can go anywhere and people can't trust what a link says.