2

I'd like to insert some user generated css into the head of my html using javascript. The issue is that when I escape certain characters break the CSS.

For example:

    let css = `
        @import url('https://fonts.googleapis.com/css?family=Roboto&display=swap');
        body { font-family: "Roboto", sans-serif; }
        `,
        style = document.createElement('style');

    style.type = 'text/css';
    document.head.appendChild(style);
    css = escape(css);
    style.appendChild( document.createTextNode( css ) );

outputs:

    <style type="text/css" id="custom-theme-styles">
    @import url(&#39;https://fonts.googleapis.com/css?family=Roboto&amp;display=swap&#39;);
    body { font-family: &quot;Roboto&quot;, sans-serif !important; }
    </style>

As you can see the the single and double quotes produce &#39; and &quot; respectively. The ampersand in the query string converts to &amp;. I could also see issues using the css child combinator (>).

Is there a way to escape the css so that it doesn't break and renders securely? It feels like a bad idea, but if I don't escape the css, is this even a security concern? I could be wrong but I don't think a <script> tag inside a <style> tag would execute. Are there any other vulnerabilities I'm missing with not escaping the css?

jwerre
  • 9,179
  • 9
  • 60
  • 69
  • I'm not sure about security concerns, but you could try `css = decodeURI(css);` What are you trying to accomplish? I'm curious if you could approach this another way without security concerns. – disinfor Oct 01 '19 at 19:38
  • I tired `decodeURI()`, same issue. It breaks the css. – jwerre Oct 01 '19 at 19:39
  • 2
    Using functions that encode URLs (or URL fragments) won't help (especially not `escape` which is pretty much deprecated). You should probably use a [CSS parser](https://stackoverflow.com/questions/10963997/css-parser-for-javascript) instead. – str Oct 01 '19 at 19:44

1 Answers1

3

I have updated this to reflect the details and guidance from the OWASP link in the comments

I think there's pretty much no benefit to using something like decodeURI() or escape() which aren't meant for this.

OWASP has a fairly comprehensive guide on XSS prevention, which includes a section on how to handle CSS.

In their own words:

CSS is surprisingly powerful, and can be used for numerous attacks. Therefore, it's important that you only use untrusted data in a property value and not into other places in style data. You should stay away from putting untrusted data into complex properties like url, behavior, and custom (-moz-binding).

You should also not put untrusted data into IE's expression property value which allows JavaScript.

It's worth reading the guide in full as there are a number of things that you need to consider as a whole, and with security it just takes one fault to undo everything.

In practice I believe you can start with passing it through something like PostCSS in order to parse the CSS first. (This also has some advantages from a general functionality/architecture point of view as it will also makes it easier to add functionality such as debug messages or CSS prefixing in future).

By passing the CSS through a parser you would harden yourself against any vulnerabilities that involve invalid CSS syntax, but more importantly it then allows you to mor eeasily filter out the problematic properties and values outlined in the guidance. E.g.

{ background-url : "javascript:alert(1)"; }

Note that OWASP strongly recommends taking a whitelist approach. Start from a point of view of disallow everything and then only allow specific exceptions when you know they are safe and necessary.

Getting XSS prevention right is non-trivial and I'm not currently aware of any tools designed to make it easier (and any such tool would have to be carefully designed and used with caution), however it also should not be prohibitively difficult.

(In an earlier version of the answer I noted that style tags were considered a text node which protects against some kinds of trivial injection attacks. While this is true the OWASP examples demonstrate that this protection is fairly illusory).

Community
  • 1
  • 1
ChrisM
  • 1,565
  • 14
  • 24
  • 4
    [Rule #0 of OWASP's XSS Prevention Rules](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-0---never-insert-untrusted-data-except-in-allowed-locations) is "Never Insert Untrusted Data Except in Allowed Locations", one of which is between ``. One avenue: `background-image: url("javascript: malware()");`. See [Rule #4](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-4---css-escape-and-strictly-validate-before-inserting-untrusted-data-into-html-style-property-values) – Heretic Monkey Oct 01 '19 at 20:22
  • Thanks @HereticMonkey, that's exactly what I was looking for. – jwerre Oct 01 '19 at 20:26
  • @HereticMonkey that's a good link. I was about to update my answer to reflect this but I feel you should get the credit. Do you want to write an answer of your own? – ChrisM Oct 01 '19 at 20:38