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

CSS injection vulnerabilities #318

Closed
jamesknelson opened this Issue Aug 23, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@jamesknelson
Copy link

jamesknelson commented Aug 23, 2017

Rebass is currently vulnerable to CSS injection vulnerabilities.

If a malicious user is able to feed props into Rebass components on a page that a target user is viewing, they can inject any styles they'd like into the target's page.

This can result in defacing your app/website, capturing keystrokes in fields, finding the characters used in a certain HTML element, or in the case of IE9, possibly even arbitrary JavaScript execution.

As an example, you can change the background color of the page via the text of a Tooltip element:

<Tooltip text='hello";} html:not(&) {background-color: blue;}  .test'>
  <Text>
    Hover Me
  </Text>
</Tooltip>

I've got a live version on codesandbox.

Using the same approach, you can execute any of the attacks linked above. Also, the problem isn't limited to tooltips; every prop that feeds into a CSS block is vulnerable.

Not sure how to approach fixing this, but given that most vulnerable props are numbers, you could strip out all double quote " and brace { } characters? This would mean reimplementing the tooltip to avoid the CSS content selector, though.

@jxnblk

This comment has been minimized.

Copy link
Collaborator

jxnblk commented Aug 31, 2017

Correct me if I'm wrong, but this is only an issue if you allow users to pass props to the React component (which you shouldn't do), right? I think the styled-components docs warn about this but I'd have to look it up

@jamesknelson

This comment has been minimized.

Copy link
Author

jamesknelson commented Aug 31, 2017

It should be ok to pass user input to props in most cases. When it isn't ok, it should be made obvious (like the dangerouslySetInnerHTML prop in React).

The styled-components docs do mention not to inject user input without sanitising it first, but that is the responsibility of the component itself, not the parent.

@jxnblk

This comment has been minimized.

Copy link
Collaborator

jxnblk commented Sep 1, 2017

I guess I've never really worked on an app where user input is being used as a prop – and sorta would guess that a responsible dev would sanitize user input before doing anything like that. Might be worth putting a note in the docs, but I have no idea how many people would do something like that. Maybe my assumption about devs being generally aware of XSS vulnerabilities isn't accurate? Regardless, I don't think sanitation belongs in a library like Rebass

@jxnblk jxnblk closed this Jun 10, 2018

@trusktr

This comment has been minimized.

Copy link

trusktr commented Sep 6, 2018

App devs should be aware, yeah, but the benefit of putting sanitation in a lib seems to be that then it doesn't matter if the devs aren't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment