Closed Bug 1292522 Opened 3 years ago Closed 3 years ago

with document.domain set on iframe/parent, permission denied on property-access across frame/parent when coming from different ports

Categories

(Core :: DOM: Core & HTML, defect)

48 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- wontfix
firefox49 + fixed
firefox-esr45 --- unaffected
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: winterfuchs_bugs, Assigned: dragana)

References

Details

(Keywords: dev-doc-complete, regression, site-compat)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160726073904

Steps to reproduce:

Build-Version:  Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
(also verified on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0 nightly from August 4th 2016)
OS: Windows 7 

Description: a parent-page and its iframe run on different sub-domains with different ports (e.g. sub1.test.local:8080 and sub2.test.local:8090). The super-domain (test.local) is the same and is set on both documents to the document.domain property in order to allow javascript-interaction between parent and iframe (described by https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy#Changing_origin) 
Since Firefox 48 a call to properties of the parent from the iframe or vice versa result in 'Error: Permission denied to access property "propertyNameHere"'. 
Before Firefox 48 the javascript-interaction was possible.

When the page and the iframe both run on the same port (but still different sub-domains), setting document.domain results in the expected behavior again.

It seems setting document.domain doesn't set the port to null anymore as was described in above linked document.

Setting one of the document.domain with the document.domain of the other including its port-number (e.g. document.domain = 'test.local:8080';) results in the javascript-interaction also working again. Sadly other browsers refuse to accept that kind of notation without error, so it's not good as a workaround.


Steps to Reproduce:

A small example with a parent and an iframe page. I misused windows' hosts-file to fake those different domain-names for my small test, but the problem orginated on 2 different servers.
The two pages set document.domain and then wait a while (to give the iframe time to load) to execute the test functions.

1.) set up parent-page on http://sub1.test.local:8080/parent.html or similiar

<!DOCTYPE html>
<html><head><title>parent</title></head>
	<body onload="document.domain='test.local';window.setTimeout(parentTest,5000);">
		<script>
			var testvar = "testparent";
			function parentTest() {
				console.log("parent '" + document.domain + "'");
				console.log("parent tests iframe '" + document.getElementsByTagName('iframe')[0].contentWindow.testvar + "'");
			}
		</script>
		<iframe src="http://sub2.test.local:8090/iframe.html"></iframe>
	</body>
</html>

2.) set up iframe-page on http://sub2.test.local:8090/iframe.html or similiar

<!DOCTYPE html>
<html><head><title>iframe</title></head>
	<body onload="document.domain='test.local';window.setTimeout(iframeTest,5000);">
		<script>
			var testvar = "testiframe";
			function iframeTest() {
				console.log("iframe '" + document.domain + "'");
				console.log("iframe tests parent '" +window.parent.testvar + "'");
			}
		</script>
	</body>
</html>

3.) ...then access http://sub1.test.local:8080/parent.html



Actual results:

With document.domain set on iframe/parent there's a permission denied error on property access across iframe/parent when iframe and parents documents came from different ports


Expected results:

Setting document.domain should probably null the port again as described in https://developer.mozilla.org/en-US/docs/Web/Security/Same-origin_policy#Changing_origin or at least allow parent/iframe-property-acces without the need to set the port too - or a workaround not clashing with other browsers would be nice too.
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=df7e8620c830db77b0d51b78ae81fcdb3d122d29&tochange=9613753f5c1c94b9ccbf0a2003913e17f69f617a

Triggered by: d81ec460382e	Dragana Damjanovic dd mozilla — Bug 409885 - Use SetHostPort in nsHTMLDocument::SetDomain. r=bz
Blocks: 409885
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dd.mozilla)
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Flags: needinfo?(dd.mozilla)
Attached patch bug_1292522.patch (obsolete) — Splinter Review
I made a minimal change so that we can uplift it to aurora and beta.

Maybe we should change SetHostPort function, but I would not uplift such a patch.
Attachment #8780554 - Flags: review?(bugs)
Olli, if you do not have time, I can give it to bz when he comes back.
Not critical enough for the 48 dot release. 
However, happy to take a patch in 49
SetHostPort has rather unexpected behavior. Why doesn't it reset port?
oh, nm, I misunderstood.
Comment on attachment 8780554 [details] [diff] [review]
bug_1292522.patch

Hmm, still rather unexpected API. Why doesn't it reset port by default?

Can we get some test for this
Attachment #8780554 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #8)
> Comment on attachment 8780554 [details] [diff] [review]
> bug_1292522.patch
> 
> Hmm, still rather unexpected API. Why doesn't it reset port by default?
> 

This function was added in bug 887364 and it is form the start implemented in this way. There is no comment in the bug why is it implemented in this way. 
I will open another bug to fix this.

> Can we get some test for this

I will write a test.
The test.
Attachment #8781608 - Flags: review?(bugs)
Attachment #8780554 - Attachment is obsolete: true
Attachment #8781610 - Flags: review+
I have open a new bug, bug 1295636, for fixing SetHostPort function.
Attachment #8781608 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06b37b01a0a5
Fix regression from bug 409885. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/af1ae450676a
Add test for bug 1292522 - the same domain host with different port numbers must be treated as the same domain. r=smaug
Keywords: checkin-needed
Duplicate of this bug: 1296259
Shouldn't the patch be proposed for branches too? This is breaking web sites.
Flags: needinfo?(dd.mozilla)
(In reply to Olli Pettay [:smaug] from comment #16)
> Shouldn't the patch be proposed for branches too? This is breaking web sites.

I was just waiting for patch to land on m.-c. before asking for an uplift. It just landed, I will ask for an uplift.
Flags: needinfo?(dd.mozilla)
Comment on attachment 8781610 [details] [diff] [review]
bug_1292522.patch

Approval Request Comment
[Feature/regressing bug #]: bug 409885
[User impact if declined]: it break some sites, I do not know how many. The uri from doc and iframe must differ only in the port number.
[Describe test coverage new/current, TreeHerder]: Test is added as well.
[Risks and why]:  low it is only one line
[String/UUID change made/needed]: none
Attachment #8781610 - Flags: approval-mozilla-beta?
Attachment #8781610 - Flags: approval-mozilla-aurora?
Comment on attachment 8781608 [details] [diff] [review]
bug_1292522_test.patch

Approval Request Comment
[Feature/regressing bug #]: bug 409885, but this is only a test.
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: no risk, this is only a test.
[String/UUID change made/needed]:
Attachment #8781608 - Flags: approval-mozilla-beta?
Attachment #8781608 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/06b37b01a0a5
https://hg.mozilla.org/mozilla-central/rev/af1ae450676a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment on attachment 8781610 [details] [diff] [review]
bug_1292522.patch

Fix for regression from 48, includes new tests. This should land in the beta 6 build today.
Attachment #8781610 - Flags: approval-mozilla-beta?
Attachment #8781610 - Flags: approval-mozilla-beta+
Attachment #8781610 - Flags: approval-mozilla-aurora?
Attachment #8781610 - Flags: approval-mozilla-aurora+
Comment on attachment 8781608 [details] [diff] [review]
bug_1292522_test.patch

Test-only patches do not need relman review, they are auto approved.
Attachment #8781608 - Flags: approval-mozilla-beta?
Attachment #8781608 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1299165
Duplicate of this bug: 1302677
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.