Inline styles

[anna-is-cute]

Firstly, looks like you released 1.0! Congrats! :D

Unfortunately, it seems that you have introduced inline styles with this update. As someone enforcing a strict Content Security Policy, the reason I was using CodeFlask is precisely because it didn't do this. Please consider changing this new update to remove the inline styles and be more CSP-friendly!

[lol768]

:+1: There are far too many JS libraries which do this and negatively impact overall security

[anna-is-cute]

Is it possible to replace this with a Sass/scss workflow? One base theme that can be customized by variables?

To make a custom theme with red tokens, for example:

@import 'variables'; // contains all base variables, including `$token`

$token: red; // overrides token to our custom color

@import 'base'; // uses the variables

[kazzkiq]

Not every user uses SASS. CodeFlask must be as much technology "agnostic" as possible. So that's out of question.

About the CSP concern, I honestly cannot see where could lie the issue with inline CSS, since:

1. It only runs only on modern browsers (which have no more issues executing crazy HTC stuff like IE had);
2. The CSS is not user-based. It is (and always will be) the exact snippet you're seeing in source-code;
3. Most (if not all) major reactive frameworks/libs use inline styles and/or CSS injection via JavaScript, and that doesn't seem like a big deal.

And of course, the main goal by injecting CSS directly via JavaScript is to make it even easier to get CodeFlask up and running. Just import it, run on some element, and you have an editor with basic highlight support.

I'm honestly interested in the threats such approach could lead to. If that's a big concern, what could be done is adding an option to prevent any CSS injection, so its up to users if they want to add all the CSS by themselves or let CodeFlask handle it.

[anna-is-cute]

What I meant is that you use Sass to generate the static CSS.

I'm sure @lol768 could explain more in depth about why inline CSS is unsafe (the CSP keyword is literally unsafe-inline), but OWASP has some good examples of how to use injected CSS to facilitate XSS attacks.

Having to make an exception and allow all unsafe inline styles to use CodeFlask means that I won't be using it. There's no reason to inject CSS when the link tag exists.

[lol768]

Hey @kazzkiq,

I personally don't care too much how the CSS is generated, I'm guessing @jkcclemens saw the interpolation and figured SASS was a good fit at built time. This is an implementation detail though, so do it however you like.

All I ask is that you please allow site owners to include the styles themselves in an external file instead of injecting it like you are now which isn't a great practice from a security perspective.

About the CSP concern, I honestly cannot see where could lie the issue with inline CSS, since: It only runs only on modern browsers (which have no more issues executing crazy HTC stuff like IE had);

I'm not sure I understand your comments concerning modern browsers. The inline CSS (which is effectively what your style injection is) is an issue in all browsers which support CSP.

The CSS is not user-based. It is (and always will be) the exact snippet you're seeing in source-code;

If it's static then there should be no problem with it being hosted in a separate file and having the user include it with a <link>.

Just to clarify, neither of us are suggesting there is a security issue in your library. Rather, the way in which you've written the library makes it incompatible with useful security controls (in the form of Content-Security-Policy).

Most (if not all) major reactive frameworks/libs use inline styles and/or CSS injection via JavaScript, and that doesn't seem like a big deal. Yeah, a lot of libraries do it and it's a major headache for someone who works in infosec. It's still an unnecessary sacrifice to have to make. We (in infosec) try and recommend the strictest possible CSP to our clients but can't when the client's project relies on libraries which do this sort of style injection because it'd cause breakage.

I can totally see why you do it. It's marginally easier for the end user, but I mean it's only an extra <link> or adding some CSS to your existing stylesheet..

---

As for why allowing unsafe-inline is a bad idea:

https://github.com/ChALkeR/notes/blob/master/Improper-markup-sanitization.md is an interesting write-up. The Vanilla issue in particular is a good example of why disallowing unsafe-inline is a good idea. Some of the other reports use classes which already exist (and a restrictive CSP wouldn't help here) but nonetheless you can see it's useful to have.

Take a look at this one, too: http://scarybeastsecurity.blogspot.com/2009/12/generic-cross-browser-cross-domain.html

I wouldn't object against an option as long as: you documented how to use it and prominently explained that if people do decide to use the style injection functionality they'll have to compromise on their CSP. I've personally wasted a lot of time dealing with libraries that it ultimately turns out aren't compatible with a secure CSP - a little documentation would've gone a long way.

And finally, thanks for actually discussing and reading my own (and @jkcclemens' reasoning). A lot of library maintainers aren't willing to look into this at all.

Cheers, Adam (@lol768)

[kazzkiq]

@lol768 Thanks for your time for a more detailed explanation. I do agree with most of your points, and those articles clarify a lot, too.

I still think the best approach here would be to give user the choice of trading ease of use for CSP comply. In this case, I believe the change could go as follows:

1. Adding csp: true|false option (defaults to false);
2. Explaining in the docs the impact of either value chosen (how it affects security if false, and how to adapt CodeFlask styles in case of true);
3. Generate CSS files do /build in case the choice was true.
4. Explaining why it is a thing and why exactly the option was added to the lib (perhaps with a tiny intro to CSP and its importance).

This way I think we can satisfy both worlds. Those who want to use CodeFlask "just for that simple weekend project" can stick with the way it works out-of-the-box now. And those who are building serious stuff and care about security can also easily adapt the library to their concerns.

[lol768]

Sounds good to me :smile:

[kazzkiq]

Great. I will implement it as soon as possible.

[valpackett]

Thanks for not removing inline, it's good for #68.

Note for everyone: you don't have to use unsafe-inline. CSP level 2 allows allowing (heh) specific inline styles/scripts via SHA hashes or nonces!

[lol768]

Note for everyone: you don't have to use unsafe-inline. CSP level 2 allows allowing (heh) specific inline styles/scripts via SHA hashes or nonces!

Yeah, and it's undesirable. Should only be used for transitioning legacy code, will leave the page broken in CSPv1 browsers and it's painful to maintain. Better to ask the library maintainers to do things properly.

Nonces wouldn't be suitable here in any case given the current implementation since the JS doesn't introduce any nonce attributes and presumably wouldn't know what the nonce was either.#

Hash might work but each script update would break the styles.

[ultimate-tester]

Did this already get implemented? I would love to make use of it, just because injecting CSS with JS is messy business