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

[docs] Add example documentation for unsafeCss attack vectors #868

Closed
dfreedm opened this issue Jan 24, 2019 · 5 comments
Closed

[docs] Add example documentation for unsafeCss attack vectors #868

dfreedm opened this issue Jan 24, 2019 · 5 comments
Assignees

Comments

@dfreedm
Copy link
Member

dfreedm commented Jan 24, 2019

Some documentation about what common security issues to be aware of, would be a nice addition

Originally posted by @kenchris in lit/lit-element#474 (comment)

@bashmish
Copy link

bashmish commented Mar 18, 2019

I think that we still need a clarification of why the css literal is so strict about arbitrary strings. It impacts DX a lot and to convince people we need to make a really good case to prove that's is needed.

I learned that this issue exists in other solutions like some of CSS-in-JS ones (e.g. in styled-components). Most of the time it is an application developer responsibility to sanitize css strings and it is not solved in any automatic way anywhere besides LitElement/lit-html (to the best of my knowledge, correct me if I'm wrong).

I would like to describe possible typical attack vectors here so that we can later on document one or a few of them for future reference. I will make a lot of assumptions here which might or might not be wrong, but otherwise the comment will be too long to follow.

My understanding is that we are talking about pure CSS attacks, or CSS injections. I'm using the terminology from here since this is one of the most well-documented list of all security issues I found on the web (and because OWASP has very limited info about CSS related attacks). Like with XSS they are of 2 types: stored (aka persisted) and reflected (aka non-persisted).

When it comes to theoretical possibility, in LitElement both can happen. Let's take a look at 2 potential vulnerability sinks:

class MyComponent extend LitElement {
  static get styles() {
    // this is a potential sink
    return css`${foo}`;
  }
  render() {
    // this is also a potential sink, but let's focus on the main recommended use-case above
    // (also because according to https://github.com/Polymer/lit-html/issues/823
    // the css literal will be likely used here as well in the future)
    return html`
      <style>${bar}</style>
    `;
  }
}

The first question I ask myself is how can attacker put malicious code into foo? Via properties passed to a component? No, because static get styles is a static getter which does not have access to runtime properties. From some globals? Yes.

Ok, so the source of attackers code is some global. But what sort of global? I know a lot of theoretical cases here, let's mention one for each types of CSS injections:

  • accessing unsatitized user inputs directly (e.g. from redux store) - stored CSS injection
  • accessing GET parameters ((new URL(window.location.href)).searchParams.get('foo')) - reflected CSS injection (requires social engineering to direct a user to a page with malicious GET parameter content)

I believe pretty much everything above is objective and clear. But to deeply understand why css literal is so restrictive I want to not only understand theoretical possibilities, but also understand practical things that people might write in LitElement-based components making them vulnerable, otherwise this is not convincing.

HONESTLY, I could not find any practical code that will contain reference to a global variables in LitElement-based components. And the main reason for this is that the styles getter is static. Since it is static, I would expect strings injected there to be static as well (I assume it does not make sense to use dynamic at all, am I wrong?). What sort of static strings will you include from globals as a developer? Really really hard to find a good example here. If someone can, I would really appreciate, for my own understanding and for future reference answering other people questions who are very disappointed with DX using LitElement's css literal. So please help to find a good meaningful code which can lead to CSS injection inside static get styles()

Next, a few words about what is possible if a CSS injection happens. Many websites on the internet focus on this topic instead of the above one. Most of the possibilities are not applicable to modern browsers thanks to many smart changes made in CSS in recent yers. Just imagine that in IE before IE10 (if I'm not mistaken) it was possible to execute arbitrary JavaScript from CSS, which is nowadays not possible. Yet there are a couple of practical cases in modern browsers which are well described in these 2 posts: "Stealing Data With CSS: Attack and Defense" and "CSS based Attack: Abusing unicode-range of @font-face". They both make a lot of sense and I believe they are severe enough to think about protecting css literals.

Last thing is whether it is a good idea to use unsafeCSS or not in your code? I don't have much to say here since I think that the answer to this is in the previous text. Looking at code using unsafeCSS you should check if it does not look like introducing any of above mentioned vulnerabilities. It limits the scope to search during the review process, which is a good thing.

So wrapping up:

  1. css literal used in static get styles is a potential vulnerability sink, so it needs protection
  2. there are 2 common CSS vulnerabilities: a stored CSS injection and a reflected CSS injection
  3. good examples showing real code open to any of above mentioned types of vulnerabilities are still to be found (help is really appreciated here)
  4. most common way to use CSS injections in modern browsers is to steal secret data either via attribute selectors or via @font-face's unicode-range.
  5. unsafeCSS helps to review code for CSS injections

@arthurevans
Copy link
Contributor

@azakus @sorvell do y'all have any thoughts on this? In particular, examples that answer @bashmish's question?

@bashmish
Copy link

bashmish commented Sep 27, 2019

Since I wrote a comment above, I learned quite a lot about security and now have a different opinion on this. Just the possibility to inject some unsanitized input into the css literal is good enough reason to implement the protection there. unsafeCSS is the right approach to solve it for such a highly reusable library which LitElement is. You can close the issue if you want.

A question: is there any guide (from Google or someone else) which I can share with my colleagues (and read myself too of course) to understand more about web security best practices? There is a lot of material on the web, for example nice articles about CSP written by Googlers, but I never saw a clear guide which developer can follow or use as a starting point.

@arthurevans
Copy link
Contributor

Hi @bashmish ... It's not my area of expertise, but I asked around. Some introductory resources:

There are also some resources on Web Fundamentals, but they're separate topics, not really a step-by-step guide that you can follow.

@aomarks aomarks transferred this issue from lit/lit-element Jun 21, 2022
@aomarks
Copy link
Member

aomarks commented Jun 21, 2022

Closing in favor of #715

@aomarks aomarks closed this as completed Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants