WICG / construct-stylesheets

API for constructing CSS stylesheet objects
Other
138 stars 25 forks source link

replaceSync accepts inline CSS which is not CSP compliant #98

Closed just-boris closed 5 years ago

just-boris commented 5 years ago

The following code currently does not work under CSP without explicit style-src: 'unsafe-inline' directive

const styleTag = document.createElement("style");
styleTag.textContent = `span { color: red;}`;
document.body.appendChild(styleTag);

You cannot create dynamic style tags, however, you can do the same thing using adoptedStyleSheets

const sheet = new CSSStyleSheet();
sheet.replaceSync(`span { color: red;}`);
document.adoptedStyleSheets = document.adoptedStyleSheets.concat(sheet);

Is this a feature or a bug in the spec?

And if this is a feature, what is the difference between these two? Why was this not allowed for the first example and allowed for second?

tabatkins commented 5 years ago

Hmm, that's a good point.

I'm not super familiar with the older discussions around CSP - is the restriction on script-created style intentional, or an accidental over-application of a rule intended to prevent parser-created style coming from the HTML? Does this also apply to <link rel=stylesheet href="data:text/css,body{background:red;}">, which does the same thing, just with more difficulty?

If it's intentional, we should restrict CSSStyleSheet similarly. If it's not, we can leave CSSStyleSheet alone, and maybe see about changing CSP to be consistent.

tabatkins commented 5 years ago

Based on a Twitter convo with @mikewest https://twitter.com/tabatkins/status/1189288783738523648, it looks like the restriction on script-created style elements is probably an accident of implementation, not an intentional restriction. Thus script-created CSSStyleSheets should be fine to stay un-restricted by CSP.

franktopel commented 4 years ago

@mikewest @tabatkins The very same behaviour described in this issue seems to still be what is currently implemented, at least in Chrome.

Can we have a final statement on this? Which of the two style generation methods will work under a CSP that has

style-src 'self';

both, either, neither?

chrishtr commented 4 years ago

@mfreed7

emilio commented 4 years ago

I think CSP doesn't block CSSOM, generally, right? That is, creating a <style> element doesn't work, but doing existingStyleElement.sheet.insertRule(...) does. So I think the current behavior (which also matches Firefox's implementation fwiw) is fine.

franktopel commented 4 years ago

[...] it looks like the restriction on script-created style elements is probably an accident of implementation, not an intentional restriction. Thus script-created CSSStyleSheets should be fine to stay un-restricted by CSP.

This was the closing comment on this issue.

How does restricting dynamic creation of <style>-tags make things any more secure when dynamic additions to the adoptedStyleSheets array can achieve the very same result?

It appears as if this has been decided the way it is merely for practicability purposes. Or does it offer any added security, allowing one way while disallowing the other?

emilio commented 4 years ago

It doesn't have any added security, but allowing script-created <style> elements while disallowing parsed-crated ones is extra implementation complexity for no benefit.