Styled-components-website: Expand Security section with a demo and notes on sanitisation

[kitten]

https://github.com/styled-components/styled-components-website/blob/master/sections/advanced/security.js

This section contains some information on Security. Right now styled-components isn't (and can't reliably) sanitise user input. This is something we can add in v3 once we can differentiate between: properties, values, mixins, selectors in interpolations.

For now we should expand the Security section with two very important things that are lacking right now:

• How serious is this problem? (demo)
• How can you fix it quickly, if your app is vulnerable?

The first problem is that we need a demo that shows the user precisely how this happens. My idea is to just link to @jamesknelson's CodeSandbox demo, or even article: https://codesandbox.io/s/llnzkwk0mz or https://reactarmory.com/answers/how-can-i-use-css-in-js-securely

The second problem is that neither our docs nor this article provide a solution, which seems to not be obvious to most. It's critical that we give users the confidence to continue using interpolations for user input when it's actually necessary.

For this we can link to CSS.escape which escapes CSS strings to make them safe for interpolations:

For this there's a new utility in JS which is in the CSSOM proposal: CSS.escape (try it out! it's in Chrome today afaik) https://drafts.csswg.org/cssom/#the-css.escape%28%29-method

There's a perfect polyfill for this method that we can use in the future by default, but for now we need to urge users to put it in their code themselves whenever they accept/interpolate user input for styled-components. https://www.npmjs.com/package/css.escape

Reference: https://github.com/styled-components/styled-components/issues/1105

[mathiasbynens]

👍

Minor suggestion: consider linking to the CSS.escape polyfill repository instead of the npm package page.

[mxstbr]

Just because I found it right now: Would you want us to recommend CSS.escape or cssesc?

I'd prefer to recommend people use the CSS.escape standard with your polyfill but I'm happy to recommend cssesc since it'll be a while before folks can use CSS.escape without the polyfill so then it doesn't really matter :wink:

[kitten]

@mxstbr Why not both? 😆 CSS.escape is nice because it's the standard, and cssesc has some nice additions to it

[mathiasbynens]

@philpl is correct: either solution does the job. cssesc offers more flexibility, in case you want fine-grained control over the output.

Generally, I recommend CSS.escape as it’s a standard now, and you get it for free in modern browser environments. (cssesc was created before CSS.escape existed.)

[jamesknelson]

Feel free to use part of the linked article in your docs (if you do, please provide a link back).

For what it's worth, I've also updated the article to mention CSS.escape and @mathiasbynens' polyfill repo.

[fgallardograzio]

Are we supposed to use CSS.escape for all user input? This issue and the current example in the docs _seems_ to suggest we do. E.g.:

const ArbitraryComponent = styled.div`
  background: url(${CSS.escape(userInput)});
`;

But in CSS.escape's docs it's only mentioned to be used to escape css selectors. That's the reason why if the escaped value starts with a number it'll escape the first character. If my url is a filename, say: 1234.png it'll return \31 234\.png. This apparently doesn't break the element's background but that's not a url I could navigate to by clicking the link in the browser's dev tools.
Or say I want to escape a user provided value for a size, e.g. 56px.
CSS.escape('56px') returns \35 6px which definitely breaks the element's styles.

Am I interpreting this incorrectly or doing something wrong?

[kitten]

If I’m not mistaken this issue (also since it’s 2 years old) has changed quite a bit in terms of the security scope we’re talking about.

On the client-side we’re using CSSOM insertion by default in production. This means that the risk of inserted CSS is lowered tremendously. In fact, all that could be inserted must be valid CSS.

So that means in production on the client-side, if you’re dealing with completely arbitrary user input (which of course is rare) you only have to deal with more being inserted than you expect.

Outside of strings in CSS that may mean escaping : and ; and curly braces for values for instance. Inside of strings maybe only quotes.

If you’re not dealing with arbitrary input then you should be fine.

If you’re dealing with SSR however you may want to ensure however that styles cannot output </style>

However that’s about it ✨

Edit: also, to define “arbitrary input”, if it’s client-side only and cannot be handed to another browser/user, you’re probably fine. If no XSS vulnerability can be triggered _without_ the user’s input then you’re not running the risk of having the site changed for anyone but the person who’s seeking to change it, which is fine.

If you’re storing arbitrary input you may want to sanitise it when it’s stored instead to avoid extra cost when using styled-components.

Only when this doesn’t apply is when you should be worrying about it in the first place 👍