Add tests to make sure data: URIs are cross origin by iframe.contentDocument

RESOLVED FIXED in Firefox 57

Status

()

P2
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: allstars.chh, Assigned: hchang)

Tracking

Trunk
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment)

Comment hidden (empty)
Status: NEW → ASSIGNED
Priority: -- → P2
Assignee: allstars.chh → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 1

a year ago
Let assume this test is for iframe.contentDocument cross-origin assurance for data:uri.
(Assignee)

Comment 2

a year ago
Instead of throwing an exception, our implementation just return null
while accessing cross-domain iframe.contentDocument:

http://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/dom/html/nsGenericHTMLFrameElement.cpp#99
Comment hidden (mozreview-request)
(Assignee)

Comment 4

a year ago
https://elefant.github.io/data-uri/iframe.html

Chrome will throw a DOMException but Firefox won't. Safari will NOT
throw exception but will print error message like

Blocked a frame with origin "https://elefant.github.io" from accessing a frame with origin "null". The frame requesting access has a protocol of "https", the frame being accessed has a protocol of "data". Protocols must match.
(Assignee)

Updated

a year ago
Assignee: nobody → hchang
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8897271 - Flags: review?(ckerschb)
(Assignee)

Updated

a year ago
Summary: add tests for bug 1364369 to make sure data: URIs are cross origin → Add tests to make sure data: URIs are cross origin by iframe.contentDocument

Comment 6

a year ago
mozreview-review
Comment on attachment 8897271 [details]
Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument.

https://reviewboard.mozilla.org/r/168566/#review174114

Hey Henry, test itself looks good, but I think we only have to update /origin-of-data-document.html inside the json, right? I know you probably ran some 'mach' command to update that json, but I am not entirely sure what implications that might have to update some other tests within that jons. Probably worth checking with jgraham.
Attachment #8897271 - Flags: review?(ckerschb) → review+
James, just to make sure, do those updates within the json file look OK to you?
Flags: needinfo?(james)
Yeah, it's just updating file hashes; the value is just used to avoid some needless work updating the actual data, so it doesn't matter too much if it's wrong and we don't lint for correctness because it creates too many makework oranges.
Flags: needinfo?(james)
(Assignee)

Comment 9

a year ago
(In reply to James Graham [:jgraham] from comment #8)
> Yeah, it's just updating file hashes; the value is just used to avoid some
> needless work updating the actual data, so it doesn't matter too much if
> it's wrong and we don't lint for correctness because it creates too many
> makework oranges.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> Comment on attachment 8897271 [details]
> Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access
> its contentDocument.
> 
> https://reviewboard.mozilla.org/r/168566/#review174114
> 
> Hey Henry, test itself looks good, but I think we only have to update
> /origin-of-data-document.html inside the json, right? I know you probably
> ran some 'mach' command to update that json, but I am not entirely sure what
> implications that might have to update some other tests within that jons.
> Probably worth checking with jgraham.

Thanks for the review and yes I was told to run |./mach wpt-manifest-update|
even I didn't create a new file. I'll try to run command again to see if
I was doing anything wrong to result in those json changes other than
origin-of-data-document.html.
(Assignee)

Comment 10

a year ago
(In reply to Henry Chang [:hchang] from comment #9)
> (In reply to James Graham [:jgraham] from comment #8)
> > Yeah, it's just updating file hashes; the value is just used to avoid some
> > needless work updating the actual data, so it doesn't matter too much if
> > it's wrong and we don't lint for correctness because it creates too many
> > makework oranges.
> 
> (In reply to Christoph Kerschbaumer [:ckerschb] from comment #6)
> > Comment on attachment 8897271 [details]
> > Bug 1365157 - wpt test cases to ensure 'data:' iframe is forbidden to access
> > its contentDocument.
> > 
> > https://reviewboard.mozilla.org/r/168566/#review174114
> > 
> > Hey Henry, test itself looks good, but I think we only have to update
> > /origin-of-data-document.html inside the json, right? I know you probably
> > ran some 'mach' command to update that json, but I am not entirely sure what
> > implications that might have to update some other tests within that jons.
> > Probably worth checking with jgraham.
> 
> Thanks for the review and yes I was told to run |./mach wpt-manifest-update|
> even I didn't create a new file. I'll try to run command again to see if
> I was doing anything wrong to result in those json changes other than
> origin-of-data-document.html.

I still got a tons of hash changes in the json files. Probably people don't
usually run the update command so that so many hash values have been out
of sync. So, should I leave those hash value changes (even some of them
have nothing to do with my patch)?
Comment hidden (mozreview-request)
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)
hg error in cmd: hg push -r . -f try: pushing to ssh://hg.mozilla.org/try
searching for changes
remote: abort: abandoned transaction found!
remote: (run 'hg recover' to clean up transaction)
abort: stream ended unexpectedly (got 0 bytes, expected 4)

Comment 14

a year ago
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67ce16cac163
wpt test cases to ensure 'data:' iframe is forbidden to access its contentDocument. r=ckerschb

Comment 15

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/67ce16cac163
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.