Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of inline styles doesn't play nice with Content Security Policy #418

Open
ctusch opened this issue Apr 7, 2016 · 6 comments
Open

Use of inline styles doesn't play nice with Content Security Policy #418

ctusch opened this issue Apr 7, 2016 · 6 comments

Comments

@ctusch
Copy link

ctusch commented Apr 7, 2016

gridstack.js seems to use inline styles extensively. We would like to disable inline styles via CSP's style-src to avoid the security risk they present. Are there any plans for gridstack.js to avoid the use of inline-styles?

@troolee
Copy link
Member

troolee commented Apr 22, 2016

What do you mean by inline styles?

All styles used by gridstack are in css files or generated inside STYLE tag. And we cannot avoid of usage of generated styles. I don't see any other way to implement all functionality gridstack provides without that.

And what kind of security risks this may cause?

@ctusch
Copy link
Author

ctusch commented Apr 26, 2016

Hey,

inline styles are the style tag and the style attribute.

I had a short look at the source and it seems like you only need the style tag in _updateStyles to set the vertical size and position. I'm not familiar enough with your project but wouldn't it be possible to do this using the DOM objects and setting their styles directly? This wouldn't violate CSP (don't ask me why exactly that is so).

E.g. change

if (this._styles._max === 0) {
    Utils.insertCSSRule(this._styles, prefix, 'min-height: ' + getHeight(1, 0) + ';', 0);
}

to

if (this._styles._max === 0) {
    var elements = document.querySelectorAll(prefix);
    for (var i = 0; i < elements.length; ++i) {
        elements[i].style.minHeight = getHeight(1, 0);
    }
}

I'm no expert on security myself but here are some risks associated with inline styles:
http://dontkry.com/posts/code/disable-inline-styles.html
http://scarybeastsecurity.blogspot.co.at/2009/12/generic-cross-browser-cross-domain.html

Please note that I'm not saying that your use of inline styles exposes these risks. But it prevents activating the CSP style-src which is a safeguard for developers to not introduce such risks by accident.

@DonnchaC
Copy link

I'd also like to be able to set a strict CSP policy and disable inline-styles. Is there any plan for Gridstack to change from using custom styles and to use DOM modification instead? 👍

@anupam-contaque
Copy link

anupam-contaque commented Dec 3, 2020

Copied from #1512:

Content Security Policy header blocks all inline-style, inline-script tags as it may impose XSS vulnerabilities.

Since, missing CSP header is identified as a critical XSS vulnerability, it would be better option to provide support for this rather than forcing users to use unsafe-inline source or removing CSP header from application.

One option is to pass server generated nonce during grid initialization which can be used as nonce attribute in inline <style> tags, that would prevent blocking of <style> tags generated by gridstack.

As mentioned in earlier comments, other option is to use javascript to set styles directly on element's style property like

document.querySelector('div').style.display = 'none';

shaurya-sisodia added a commit to iongroup/gridstack.js that referenced this issue Jun 13, 2022
… Security Policy

* updating styles directly on HTMLELement rather than updating dynamic stylesheets
* removing logic related to dynamic stylesheets as they aren't required and violate CSP sctrict style-src rule
* removed UTs related to Dynamic stylesheets
shaurya-sisodia added a commit to shaurya-sisodia/gridstack.js that referenced this issue Jun 20, 2022
shaurya-sisodia added a commit to iongroup/gridstack.js that referenced this issue Jul 19, 2022
… Security… (#1)

* updating styles directly on HTMLElement rather than updating dynamic stylesheets
* removing logic related to dynamic stylesheets as they aren't required and violate CSP strict style-src rule
* removed UTs related to Dynamic stylesheets
* using adopted style sheet and replaceSync to update dynamic css rule in csp compliant manner
* Updating styles on dirty nodes elements directly in case ConstructableStylesheet is not supported
shaurya-sisodia added a commit to shaurya-sisodia/gridstack.js that referenced this issue Jul 21, 2022
@adumesny
Copy link
Member

adumesny commented May 1, 2023

nonce was added as part of #2229 - marking as won't fix for the laternative way to set inline styles instead.

@adumesny
Copy link
Member

adumesny commented Feb 9, 2025

this will get solved by #2854 I believe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants