Closed Bug 1387983 Opened 5 years ago Closed 5 years ago

Test case to ensure data:stylesheet to be considered same origin

Categories

(Core :: DOM: Security, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: hchang, Assigned: hchang)

References

Details

Attachments

(1 file)

The current test case to check if data:stylesheet is considered 
same origin [1] is wrong. CSS can be loaded from whatever domain.

One of the cases where css origin matters is accessing document.styleSheets[0].cssRules [2]. This bug is to correct the
existing test.


[1] http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/dom/base/test/test_data_uri.html#123
[2] http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/StyleSheet.cpp#720
Blocks: 1365145
Assignee: nobody → hchang
Status: NEW → ASSIGNED
Attachment #8894441 - Flags: review?(ckerschb)
Comment on attachment 8894441 [details]
Bug 1387983 - Fix test case for data:stylesheet same origin check.

That looks correct to me, but I am not a dom/ peer, hence I think it makes sense to have smaug sign off on it.
Attachment #8894441 - Flags: review?(ckerschb)
Attachment #8894441 - Flags: review?(bugs)
Attachment #8894441 - Flags: feedback+
My test page:

https://elefant.github.io/data-uri/css.html

== What firefox would print ==

haha:data:text/css,.green-text{color:rgb(0,%20255,%200)};
haha:[object CSSRuleList]

haha:https://www.w3schools.com/Tags/styles.css
SecurityError: The operation is insecure.

haha:https://elefant.github.io/data-uri/styles.css
haha:[object CSSRuleList]

Chrome and Safari don't seem to throw exception but just return null.

p.s. Chrome and Safari will only load data:stylesheet in secure context.
Comment on attachment 8894441 [details]
Bug 1387983 - Fix test case for data:stylesheet same origin check.

https://reviewboard.mozilla.org/r/165620/#review170790

This is less about DOM, but CSSOM. I think dbaron or someone should review.
Could you link to the relevant spec where this behavior is defined.
Attachment #8894441 - Flags: review?(bugs)
IIRC Yoshi have pasted the spec regarding 'data:stylesheet' same origin property
somewhere (I am looking for it right now or Yoshi can point that out, too). 

As for the security check for 'document.styleSheets.cssRules':

Described in

https://www.w3.org/TR/2016/WD-cssom-1-20160317/#the-cssstylesheet-interface

"The ownerRule attribute must return the owner CSS rule. If a value other than null is ever returned, then that same value must always be returned on each get access.

The cssRules attribute must follow these steps:

If the origin-clean flag is unset, throw a SecurityError exception.
Return a read-only, live CSSRuleList object representing the CSS rules."


[1] http://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/StyleSheet.cpp#720
Flags: needinfo?(allstars.chh)
Attachment #8894441 - Flags: review?(cam)
Hi Cameron,

Could you help review the patch made for ensuring data:stylesheet has
the same origin as the owner document? You can refer to [1] to find the 
spec for the data:css same origin things.

As for the cssRules security check, you can find the spec in comment 6 or

https://www.w3.org/TR/2016/WD-cssom-1-20160317/#the-cssstylesheet-interface

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1373513#c13
Flags: needinfo?(allstars.chh)
Comment on attachment 8894441 [details]
Bug 1387983 - Fix test case for data:stylesheet same origin check.

https://reviewboard.mozilla.org/r/165620/#review171042

Thanks, this change looks good to me.
Attachment #8894441 - Flags: review?(cam) → review+
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4923da1e6f62
Fix test case for data:stylesheet same origin check. r=heycam
(In reply to Cameron McCormack (:heycam) from comment #8)
> Comment on attachment 8894441 [details]
> Bug 1387983 - Fix test case for data:stylesheet same origin check.
> 
> https://reviewboard.mozilla.org/r/165620/#review171042
> 
> Thanks, this change looks good to me.

Thanks Cameron! I'm so happy to get your first r+ :)
https://hg.mozilla.org/mozilla-central/rev/4923da1e6f62
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Henry, thank you for fixing this so quickly!
And thank you, Cameron, for the prompt review.  :)
See Also: → 1381744
You need to log in before you can comment on or make changes to this bug.