openstyles / stylus

Stylus - Userstyles Manager
https://add0n.com/stylus.html
GNU General Public License v3.0
5.32k stars 300 forks source link

Prevent CSS password extraction attacks #385

Open hyperfekt opened 6 years ago

hyperfekt commented 6 years ago

https://www.mike-gualtieri.com/posts/stealing-data-with-css-attack-and-defense
https://github.com/maxchehab/CSS-Keylogging
Can we do something that's not all too involved to disable this exploit? This could not only theoretically, but easily be used for basically extracting an entire password. (It only works on sites that use a framework that updates the value attribute with the actual value, but that is the case for many sites today as for example react is one of those.)

Mottie commented 6 years ago

If you go to Google's sign-in page you'll see that the "Email or phone" and "password" inputs have a data-initial-value attribute which is actively updated during input. Someone on userstyles.org was creating styles using this vulnerability to capture the username and password...

I reported this issue to Google back in Feb. This is the response I got:

Changed status: New → Intended Behavior

mh...@google.com added comment #4: Hey,

Thanks for your bug report and research to keep our users secure! We've investigated your submission and made the decision not to track it as a security bug.

Personally, I'm appalled, but I'm not sure it's something that needs to be handled within the userstyle manager... I'll leave that up to the boss though.

DanaMW commented 6 years ago

wow @ google

hyperfekt commented 6 years ago

The thing is that usually this is not a problem, as the sites control the CSS they are styled with instead of it coming from untrusted sources, that's why it's probably not in their threat model. But userstyle extensions change this, and people assume by default (usually at least somewhat correctly) that CSS will not be able to perform such powerful attacks. I do believe that it is the responsibility of both extensions and hosting sites to prevent such attacks (and possibly of browsers that provide a separate user sheet API, but they move a lot slower and I'm not sure which ones those are -> I haven't searched the bugtrackers yet)

DanaMW commented 6 years ago

I would be against an extension author restricting what I can do with it at a given site. Not unlike all these people that attempt to make theirs not work on google I would alter it to put it back so it does work . So that would leave style sites.

hyperfekt commented 6 years ago

I disagree severely. In the default case it's not you doing this to a site, but a random stranger that happened to make a userstyle you liked. Even if it turns out anyone needs the capability to style password fields with images based on their attributes, this should only be allowed by a flag to be set by the informed user. We should also think about the fact that this allows not only password harvesting, but basically any entered sensitive information. In general this presents a huge security risk that I hope will not be underestimated.

DanaMW commented 6 years ago

severely huh, random strangers don't do anything on my machine. With that being said your solution would be a fair compromise to me but a blanket restriction, if the decided upon solution would just get ripped out in my case, if i even continued using it. Site that host the exploitation are the contact point for the uninformed users so why should my software be limited because they are unaware. That is just my OPINION humbly offered.

tophf commented 6 years ago

The exploit doesn't work on sites with proper (strict) CSP that allows only known resources to be loaded by CSS. Anyway, that's something I also contemplated. Ideally we should detect this.

narcolepticinsomniac commented 6 years ago

Well, usercss aside, repos are ultimately responsible, but US.o is a joke. They had an influx of new styles abusing this exploit recently. Took weeks for them to even delete the styles which were archived by the mod, and they never implemented any kinda check for the exploit in the submission parser AFAIK. I think the only reason they stopped is because the mod was actively checking new styles and archiving them, so it wasn't worth their while.

I don't think we have some responsibility to implement a safeguard, but it'd be nice if we did. Before anyone wastes any time on this, we should probably come to a consensus on the details. We'd need to agree what to check for, when and where to check, and what action to take if a match is found.

As for what to check for, the examples we've seen have been obvious, using input[type], but they could just as easily use a class or id. Only consistent thing seems to be "value" in the attribute, so we could check for a string with that included in any selector, followed by opening/closing curly brackets with any URL. Should be specific enough to find what we're looking for without false positives.

AFAIK, the only real CSS vulnerability is external URLs, and the only serious security issue is the password hack. That's not to say they can't be abused elsewhere, as a malicious ping for whatever purpose. Not as big a deal, but we could discuss addressing other potential red-flags, like shortened links or URLs with parameters.

I agree with Dana that we shouldn't do anything drastic like force deletion or preventing installation. My first thought regarding checking and what action to take, is that something similar to the "bad regexp" detection would probably be ideal, and we could probably incorporate it into the same UI. When a style triggers the check, it is disabled by default. The popup would show the same type of warning, with the only difference being that there'd need to be an option for the user to override the warning and have that pref remembered for that style.

hyperfekt commented 6 years ago

I am not sure which techniques this extension uses in different browsers, but as I hinted at earlier, if they offer a specific API for user style sheets maybe they could be convinced to be responsible for implementing a defense against this, especially since they seem better equipped at detecting this with a minimum number of false positives. My hopes are rather slim though.

mikhoul commented 6 years ago

I follow this discussion and I have seen that there is an extension for Chrome/Firefox to protect users, maybe it could be useful to look at it: https://github.com/mlgualtieri/CSS-Exfil-Protection

nyanpasu64 commented 6 years ago

Do you have any specific examples of keylogger styles, or how the attack works? Also how would I defend against attacks? Avoid anything which targets * sites?

mikhoul commented 6 years ago

@jimbo1qaz @tophf A very simple way would be to implement a "manual" scan in Stylus in the manager window that could scan the CSS to see is there is potentially malicious selectors that refer to ana external resources like background:url("https://attacker.host/badware.php"); the user could at least take a look at the code before using the style.

A simple button named something like "Scan for potentially external malicious selectors" would help to filter 90% of the possibly bad userstyles. After it would be up to the user to use or not use the suspect userstyle.

A good template for scanning the Userstyle would be: https://github.com/mlgualtieri/CSS-Exfil-Protection/blob/master/chrome/content.js

Regards :octocat:

tophf commented 6 years ago

We already have a CSS parser so I think it'll be trivial to add the check. It's just that I'm lazy and no one else cares, apparently. I also think the check should be performed automatically, no extra buttons.

mikhoul commented 6 years ago

@tophf I said "manually" because some advanced users don't want a popup to happen each time an external resource is suspected to see a popup to warn them, at least it is what I read in this thread in previous posts.

For me in any software it is always a big plus to have the choice to enable or disable advanced options. For newbies that begun with CSS and take their styles from various websites it would be kind for them to have such scan to warn them about potential malicious selectors.

I understand it's not a high priority but it would be a good feature to add.

Regards :octocat:

tophf commented 6 years ago

No one knows how the final result will look like yet but I believe there'll be no need for options or buttons because the detection won't have false positives.

myfonj commented 6 years ago

Probably just presenting summary of all external resources (url()s) that could be potentially loaded by the userstyle could give it's user quite clear picture what he can expect. Zero external resources: OK. Few images, used in content or background of simple selectors: probably OK. Gazillion pictures linked to attribute selectors: red flag. (At this moment I'm not sure whether @import is already forbidden, but I assume it is.)

I think it would make such attack much harder if Stylus just blindly loaded and forcibly cached all those resources at once during style install (so no request would be sent to attacker after that) and refreshed them only with update check (again, all at once).

narcolepticinsomniac commented 6 years ago

I think it would make such attack much harder if Stylus just blindly loaded and forcibly cached all those resources at once during style install (so no request would be sent to attacker after that) and refreshed them only with update check (again, all at once).

Not only sufficient for security, but better speed. I've wondered if it'd be worth it for speed alone. I guess in extreme cases that might take up a lot of space though. Not sure if that'd be prohibitive on some devices.

myfonj commented 6 years ago

Yes, it could have some potential benefits not related to security as well, at least protection from resource down times and reduced flicker of dynamically loaded ones. Some extra performance and/or memory consumption overhead will be unavoidable i'd assume.

When I wrote that Idea, I haven't clue how it could possibly work, how to feed userstyle external resource from different (local) source without it noticing … some service worker magic proxy perhaps? … but there is one brain dead simple one forgot, although we have been using it for ages already: just convert those URIs to dataURIs. Not nice to look at resulting CSS, even more if same resource is used several times, some +1/3 "binary" data size increase, murky outcome for SVGs with external dependencies (being probably another attack vector) but overall nothing arcane.

eight04 commented 5 years ago

It is hard to scan external resources if the style was written with preprocessors since they can generate dynamic URLs based on options or even random numbers.