Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Inline CSS vs. build to separate CSS file #752

Open
anders-kiaer opened this issue Feb 7, 2020 · 4 comments · May be fixed by #753
Open

Inline CSS vs. build to separate CSS file #752

anders-kiaer opened this issue Feb 7, 2020 · 4 comments · May be fixed by #753

Comments

@anders-kiaer
Copy link

CSP (Content Security Policy) is a good tool for enhancing security of web applications. Mozilla Observatory is a good place to check for CSP implementation across web applications. How easy it is to enforce strict CSP settings depends, to a large degree, on the frameworks used in the stack.

Dash out of the box is quite CSP settings friendly, e.g. you can do pip install dash flask-talisman (alternatively set the CSP headers directly instead of using flask-talisman) and then run e.g.

import dash
import dash_html_components as html

from flask_talisman import Talisman

app = dash.Dash(__name__)

CSP = {
    "default-src": "'self'",
    "script-src": [
        "'self'",
        # Due to https://github.com/plotly/dash/issues/630:
        "'sha256-jZlsGVOhUAIcH+4PVs7QuGZkthRMgvT2n0ilH6/zTM0='",
    ]
}

Talisman(app.server, content_security_policy=CSP, force_https=False)

app.layout = html.Div(children=["Hello Dash!"])

if __name__ == "__main__":
    app.run_server()

This will work with no CSP errors on localhost in the browser console - despite quite strict CSP settings.

If however import dash_core_components as dcc is added, you will need to either add

    "style-src": ["'self'", "'unsafe-inline'"],

or

    "style-src": [
        "'sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='",
        "'sha256-cJ5ObsneXZnEJ3xXl8hpmaM9RbOwaU6UIPslFf0/OOE='",
        "'sha256-joa3+JBk4cwm5iLfizxl9fClObT5tKuZMFQ9B2FZlMg='",
        "'sha256-Jtp4i67c7nADbaBCKBYgqI86LQOVcoZ/khTKsuEkugc='",
        "'sha256-MoQFjm6Ko0VvKSJqNGTl4e5H3guejX+CG/LxytSdnYg='",
        "'sha256-kkTbqhXgCW2olAc5oz4XYZtzEwh5ojnaxt9iOTAZcZo='",
    ],

to the CSP dictionary in order to allow the CSS that dcc is adding inline to the app.

The first is not optimal as you then again open for inline CSS "XSS" (as when not using CSP at all). The second is not optimal either as the hashes will need to update each time a new version of dcc changes its inline style content.

If dcc could output CSS as a separate file during build, instead of inline style, we could with Dash also enforce strict CSS CSP. I.e. use mini-css-extract-plugin instead of style-load in webpack.config.js during (production?) build of dcc.

@anders-kiaer anders-kiaer linked a pull request Feb 7, 2020 that will close this issue
@alexcjohnson
Copy link
Collaborator

We also got this request a while ago for plotly.js plotly/plotly.js#2355 - and at least there it seems like there's a theoretical solution by converting from styles to presentation attributes, since it's SVG. Here and in dash-table we don't have that option, but I guess most use cases - assuming we were to pull the hardcoded inline styles out of this package and into style sheets - could be handled by users via classes and their own style sheets.

I have to say though I just don’t get what the attack vector is. I understand from reading your link how after malicious CSS is on the page there’s a problem. I just don’t see how an attacker would inject their own CSS into the page, and how preventing inline styles guards against this injection. Can you provide any more information about this?

@anders-kiaer
Copy link
Author

Thanks for your reply @alexcjohnson

Can you provide any more information about this?

I'm not an expert on this matter, but can try. Typical attack vectors I guess would be the same as for JavaScript cross-site scripting (XSS), including both non-persistent (reflected) and persistent (stored) XXS. The former typically being triggered by a malicious user sending a specially crafted URL by e.g. e-mail to the victim, or through social engineering. The latter by the attacker being able to store malicious user input on the server which is later rendered for all other users of the app (typical example here is e.g. a forum / discussion board). Both these attack vectors rely on some code in the application not (correctly) sanitizing user input.

In a perfect setting where all user input is sanitized correctly/safely, disallowing inline JavaScript and CSS using CSP would not make a big difference from a security point of view (if at all). However, in a complex (Dash) web app, this could be hard to guarantee. Especially with a growing pool of community provided Dash components. It is potentially enough that one community provided component is used, which e.g. changes inline style based on some non-sanitized user provided input. Inline style vulnerabilities are not so well known to developers as compared to e.g. inline script, increasing the chance of e.g. taking style input sanitizing not so seriously.

Being able to set strict CSP settings, including disabling inline script and style, enables developers to know that certain types of vulnerabilities (either in own code or dependencies) would not be exploitable, or at least much more difficult to exploit.

Here and in dash-table we don't have that option.

Even when disallowing inline style through CSP you can set individual style attributes dynamically on elements:

document.getElementById("some_id").style.backgroundColor = "green";

Not sure if that is enough for dash-table (but I would expect it)? That this is allowed in a CSP setting can be tested by e.g. running

import dash
import dash_html_components as html

from flask_talisman import Talisman

app = dash.Dash(__name__)

csp = {
    "default-src": "'self'",
    "script-src": [
        "'self'",
        "'sha256-jZlsGVOhUAIcH+4PVs7QuGZkthRMgvT2n0ilH6/zTM0='",
    ]
}

Talisman(app.server, content_security_policy=csp, force_https=False)

app.layout = html.Div(id="some_id", children=["Hello Dash!"])

if __name__ == "__main__":
    app.run_server()

and then in the browser console run that single line of JavaScript.

However running e.g.

document.getElementById("some_id").setAttribute("style", "font-size: 200%");

in the console will be blocked by CSP since the style attribute is set directly (another example in this SO post). Probably because it resembles inline style tags <style>...</style>, and thus also being more easily available for general style injection. There are some more thoughts about that difference in this StackOverflow answer and this webappsec-csp GitHub issue.

Looks like with react >= 15, React also uses the CSP compatible way of setting individual style properties, such that you can write React code with style={{someattr: somevalue}} and still be compatible with strict CSP. See facebook/react#5878 (comment).

Some more references:

@alexcjohnson
Copy link
Collaborator

Thanks @anders-kiaer - makes a lot more sense to me now.

Even when disallowing inline style through CSP you can set individual style attributes dynamically on elements

That makes this all a lot more manageable! If D3 is implemented in a compatible way as well, that would make the plotly.js variant much more tractable too.

@anders-kiaer
Copy link
Author

Sounds good @alexcjohnson.

Yes, the selection.style(...) method on d3 selections should also be CSP compatible since, looking at the d3 source code, the statement

d3.select("#some_id").style("background-color", "green")

should be ~equivalent to

document.getElementById("some_id").style.setProperty("background-color", "green")

which as expected is also valid in a strict CSP setting (I double checked now using the Dash example above).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants