Closed Bug 602286 Opened 12 years ago Closed 5 years ago

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


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






(Reporter: ivo.wetzel, Unassigned)


(Blocks 1 open bug)


(Whiteboard: [necko-backlog])


(1 file, 1 obsolete file)

557 bytes, text/html
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 = ''
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 upon leaving the page.

Expected Results:  
Any actions in the onclose event that affect the location should be ignored when leaving the apge.
Attached file Redirect demo (obsolete) —
Basic demo of the bug in action. Click the google link, you'll end up at yahoo.
Attached file 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.
Attachment #481296 - Attachment is obsolete: true
blocking2.0: --- → ?
Component: General → DOM: Mozilla Extensions
Ever confirmed: true
QA Contact: general → general
Honza, care to look into this? We prevent loading other urls from onunload, we should do the same here.
Assignee: nobody → honzab.moz
blocking2.0: ? → betaN+
Assignee: honzab.moz → nobody
Component: DOM: Mozilla Extensions → Networking: WebSockets
QA Contact: general → networking.websockets
Assignee: nobody → honzab.moz
Depends on resolution of bug 616733 if this should block or not.  removing the flag for now.
blocking2.0: betaN+ → ---

Since WS is re-enabled now, we need to fix this.  I can take it if you're too busy.  Let me know.
(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.

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.
I am less familiar with the user-facing security aspects of the browser, so I will defer to Jesse and Dan.
Whiteboard: [sg:?]
Blocks: eviltraps
Whiteboard: [sg:?]

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)
Not urgent. I'm pretty sure there are other ways to keep from being navigated away from (see the "eviltraps" metabug, bug 432687).
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.
Releasing, any one feel free to take it.
Assignee: honzab.moz → nobody
Assignee: nobody → jduell.mcbugs
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?
Assignee: jduell.mcbugs → nobody
Hopefully this will be fixed by the same mechanism as bug 391834.
Whiteboard: [necko-backlog]
Bulk change to priority:
Priority: -- → P1
Bulk change to priority:
Priority: P1 → P3
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.
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.