-
-
Notifications
You must be signed in to change notification settings - Fork 701
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
Does this handle CSS? #32
Comments
In its current state: no. a) A CSS parser in JS that matches what browsers do individually Unfortunately, there's no unified DOM API yet (afaik) that allows us to get there - or something that allows to easily and safely get all rules from a certains tylesheet and iterate over them. If there was one, we could actually consider. |
@joelabair So, I did some research and found out that we can indeed handle CSS sanitation in a clean and cross-browser compatible way. Now, what I would like to know before starting an implementation: What kind of sanitation would you need? How do you imagine config and purpose? |
There's a multitude of stuff I wouldn't want to allow on user provided styles. The least would be to disallow URIs and Incomplete list of interesting things:
Thanks for DOMPurify! |
@graste Thanks! So, here's what I think is possible. Let's assume the following files: //test.html
<link rel="stylesheet" href="test.css">
<p>blafasel</p> /* test.css */
* {
color: blue;
background: red;
} What we can do to find out about CSS property-value details is the following (very ugly code, demo only): for(var i in styles = document.styleSheets) {
for(var j in styles[i].cssRules) {
for(var k in rules = styles[i].cssRules[j]) {
if(rules[k] instanceof CSS2Properties){
for(var l in rule = rules[k]) {
if(isNaN(l) && rule[l] && typeof rule[l] === 'string') {
console.log(l + ':' + rule[l])
}
}
}
}
}
}; So, as you can see we can go over all CSS property-values and decide if we like them or not - by whatever criteria we choose. After that we simply assemble a new style sheet and throw it into the sanitized document. Done. No risk for browser weirdness, no massive overhead. So we don't have to parse, use regex or anything to get hands on the important data. Now the tricky question is: What do we validate against? A white-list of properties? A conformity pattern for values? A conformity pattern for selectors? Depending on the context, different requirements might exist. And once we have them - how do we allow users to configure them? Technically, CSS sanitation is possible in the DOM. But what API do we offer to do it? |
No fast answers here. The obvious thing would be a whitelist of properties. I'm not sure about sanitizing property values as I'm not sure if browsers skip weird values already (I'd guess so, though). Allowing only certain values for certain properties would be cool, but I'm not sure if that's worth the effort or if devs even want to configure something like that. How about some sensible defaults or config options like this:
tl;dr: I'm really not sure. Except for disallowing |
I am tempted to think we need a new project for this - CSSPurify :) |
:-) |
So, after having a bit of a look at how browsers use the CSSOM some new problems arose. I was hoping that upon accessing the CSSOM's properties, the user agent would normalize so a benefit of a potential CSSPurify would be not to be forced to deal with obfuscated CSS. I was wrong. Here's an example: <!-- test.html -->
<style>
* {
color: red;
background: blue;
border: /**/C\41lc(10px + 10px) solid green;
}
</style>
<p>000</P> console.log(document.styleSheets[0].cssRules[0].style.border)
So you can see, browser implementations don't seem to be ready for a CSSPurifer yet. Alas we are talking about a very strict whitelist. Thoughts on this are very welcome ;) |
It gets better. Chrome cannot deal with the CSS escape inside the property name. The result of the JavaScript call? Watch closely:
|
Yes, please do so, we could add a hook to get this into DOMPurify later on. I'd love to keep the focus on this project. ;) |
Awesome examples with calc. I would've guessed (hoped) there are better parsers in browsers nowadays. :-) |
Locking down position to static would also be desirable, as restricting width and height to maximum values. These enable attackers to create overlays, which hijack clicks and redirect users to phishing pages. |
Please file bugs for the browser bugs you find. Thx |
I decided to close this: DOMPurify will not support sanitizing CSS. I am however open to contribute to a CSSPurify project is anyone is interested to set one up. I believe there is a need and we have enough knowledge here to be able to do it. |
@cure53 is there any place where current state of CSS sanitation mentioned? Am I not safe by default, doing DOMPurify.sanitize() on, say |
Yes, CSS-based attacks are not part of our threat model. We prevent XSS and DOM Clobbering. |
Does DOMPurify sanitize CSS style sheets embeded in the markup?
The text was updated successfully, but these errors were encountered: