Skip to content
This repository has been archived by the owner on Jul 30, 2019. It is now read-only.

window.open(url, name) is vulnerable to XSS with "name" collision #262

Closed
arronei opened this issue Apr 26, 2016 · 6 comments · Fixed by #1406
Closed

window.open(url, name) is vulnerable to XSS with "name" collision #262

arronei opened this issue Apr 26, 2016 · 6 comments · Fixed by #1406

Comments

@arronei
Copy link
Contributor

arronei commented Apr 26, 2016

Xiaoran Wang ([email protected]) This is a joint research with Travis Safford.
window.open(url, name, [args]) makes it easy for websites accepting user supplied URLs to be vulnerable when attackers can cause a collision on the the “name” parameter. For example, consider a genuine site embedding an iframe with name “myFrame”, and allows user to supply the url of the iframe.

e.g. www.example.com/myPage

<iframe src=”http://www.google.com/” id=”myFrameId” name=”myFrame”></iframe> <script> window.open(user_supplied_url, “myFrame”); </script>

In normal cases, if the user_supplied_url is a regular http url, the frame would be redirected to the new URL. Otherwise if the user_supplied_url is a “javascript:” script, an exception would be thrown because of the Same Origin Policy (SOP).

However, if an attacker frames this page on his own site and names the frame “myFrame” as well, the URL will be opened in the parent frame instead of the frame within the genuine site. In fact, if the url is “javascript:”, it will execute on the genuine domain causing an XSS.

e.g. www.attacker.com

<iframe src=”http://www.example.com/myPage” name=”myFrame”></iframe>

You can also test this out with Test3 at “http://test.attacker-domain.com/sopresearch/windowopen.html”. This page assumes that “test.attacker-domain.com” is the malicious domain while “exam.attacker-domain.com” is the genuine domain. You can observe that JavaScript runs in the genuine domain when it’s framed by the malicious domain with the same “myFrame” name.

The reason this happens is because whenever there are multiple browser contexts with the same “name”, the spec does not enforce the order of selecting the browser contexts. In fact, the spec says “the user agent should select one in some arbitrary consistent manner” which looks very vague as a spec. The full spec on searching for the browser context for the “name” is below.

“If the given browsing context name is not _blank and there exists a browsing context whose name is the same as the given browsing context name, and the current browsing context is familiar with that browsing context, and the user agent determines that the two browsing contexts are related enough that it is ok if they reach each other, then that browsing context must be the chosen one. If there are multiple matching browsing contexts, the user agent should select one in some arbitrary consistent manner, such as the most recently opened, most recently focused, or more closely related.”

It is not intuitive for the parent frame to be chosen as the browsing context when there is a child iframe with the same name in the current page. Not only does this potentially present XSS vulnerabilities, but it makes pages render differently when iframed. We would recommend the spec enforce the searching order from the bottom up, from within the current page, and reach out to parent or outer pages if nothing is found within the page. This would limit the impact of sites becoming vulnerable when accepting a user supplied url to the URL parameter of window.open.

@arronei
Copy link
Contributor Author

arronei commented Apr 26, 2016

@scunliffe
Copy link

I'm not sure if this is related, but IE (not sure if Edge fixes this yet) also doesn't have a separate context per site. As such if 2 sites load content into a window named "chooseitem" they will both write to the same window. This is quite commonly hit in development between local server, QA, UAT, and Production environments.

@plehegar plehegar changed the title Security: window.open(url, name) is vulnerable to XSS with "name" collision window.open(url, name) is vulnerable to XSS with "name" collision May 10, 2016
@travisleithead travisleithead added this to the HTML 5.2 WD 4 milestone Oct 24, 2016
@LJWatson LJWatson modified the milestones: HTML 5.2 WD 4, HTML 5.2 WD 5 Jan 17, 2017
@LJWatson LJWatson modified the milestones: HTML 5.2 WD 5, HTML 5.2 WD 6 Feb 26, 2017
@chaals chaals assigned adanilo and unassigned plehegar Apr 3, 2017
@chaals chaals modified the milestones: HTML 5.2 WD 7, HTML 5.2 WD 6 Apr 3, 2017
@LJWatson
Copy link
Collaborator

Should this issue be labelled "substantive"?

@chaals
Copy link
Collaborator

chaals commented Feb 2, 2018

Seems like the sound solution would be as recommended:

We would recommend the spec enforce the searching order from the bottom up, from within the current page, and reach out to parent or outer pages if nothing is found within the page

This is a change to the browsers chapter... in the last sentence of step 4.

@chaals chaals assigned chaals and unassigned siusin Feb 2, 2018
@chaals chaals modified the milestones: HTML5.3 WD1, HTML5.3 WD2 Feb 2, 2018
@chaals
Copy link
Collaborator

chaals commented Feb 2, 2018

Moving to the next milestone. See also #132 and ongoing discussions starting at whatwg/html#1440

@LJWatson LJWatson modified the milestones: HTML5.3 WD2, HTML5.3 WD3 Mar 13, 2018
@ylafon
Copy link
Member

ylafon commented Apr 16, 2018

See also whatwg/html#1509 (comment) (and subsequent msg)

chaals added a commit that referenced this issue Apr 24, 2018
First cut at matching Firefox/Chrome implementation
fix #262
siusin pushed a commit that referenced this issue Apr 25, 2018
* only select browsing contextt by name within unit of related...

First cut at matching Firefox/Chrome implementation
fix #262

* fix the link I broke

* sorry, typo

It's good to get people's names right when thanking them.

* typo

(although either way made sense, fixing it gives the intended text)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.