Closed Bug 1292522 Opened 8 years ago Closed 8 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)

Product:
Core
Component:
DOM: Core & HTML
Version:
48 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla51
Tracking Flags:
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)

bug_1292522_test.patch
4.06 KB, patch
smaug
: review+
Details | Diff | Splinter Review
bug_1292522.patch
1014 bytes, patch
dragana
: review+
lizzard
: approval-mozilla-aurora+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
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
status-firefox48: --- → affected
status-firefox49: --- → affected
status-firefox50: --- → affected
status-firefox51: --- → unaffected
status-firefox-esr45: --- → unaffected
Ever confirmed: true
Keywords: regressionwindow-wanted
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
status-firefox48: affected → wontfix
tracking-firefox49: --- → +
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
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: 8 years ago
status-firefox51: affected → fixed
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?
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: