SpacingBat3 / WebCord

A Discord and SpaceBar :electron:-based client implemented without Discord API.
MIT License
1.92k stars 95 forks source link

Fix the Chromium Extensions feature #419

Open hollowshiroyuki opened 1 year ago

hollowshiroyuki commented 1 year ago

Is your feature request related to a problem? Please describe. The Feature.md file specify that WebCord will load unpacked Chromium extensions found in {userData}/Extensions/Chromium/{extension name}/ but that's currently not the case for two reasons :

It's listed as an experimental feature along with custom styles which are working on standard builds so I guess it should be enabled as well ?

Describe the solution you'd like Remove the guard and fix the path in the code. In my opinion it's best to have the directory named Chromium instead of Chrome but it's not that important.

Describe alternatives you've considered I understand that allowing Chromium extensions can be a security concern and there's currently no setting available to toggle them so editing the Feature file to the right path and specify it's only enabled in devel builds is a possible fix.

I didn't submit a pull request as I don't know which solution follows the wishes of the maintainer, if you tell me what would be better I'll happily submit a fix.

Additional context No further context is required.

SpacingBat3 commented 1 year ago

In my opinion it's best to have the directory named Chromium instead of Chrome but it's not that important.

It's probably better to leave it as-it-is, but properly document it.

(…) It's [Chromium extensions feature] listed as an experimental feature along with custom styles which are working on standard builds so I guess it should be enabled as well?

I don't think this should be done, while CSS is experimental as well WebCord both can do a lot to prevent them from loading without user consent and even now CSS feature is much more reliable than Chromium extensions. In fact, the lack of support on Electron side is what seems to be killing this feature.


That being said, I'll still reconsider if I really want to bundle the protections that verifies if the user adds the extension or automated script, I don't think any browser does protect locally-saved extensions in any way and verifies if these are legitimate or not. And Firefox definitely doesn't prevent any 3rd-party program from messing with its config or even internal theming. This brings a question if I shouldn't just make these features optional and instead of enforcing it as a security policy, make it more relaxed feature toggle-able by build flag as optional client hardening step and adapt it a bit slower.

czkz commented 10 months ago

Hi! I need to inject a script into WebCord to stream with audio (on Linux). I currently do this manually via DevTools, but an extension would be a perfect fit to do this automatically. It seems a little overkill to patch and recompile WebCord or to move to a less stable version just to get access to this feature. Perhaps extension support should be enabled in developer mode, since it's so similar to DevTools in spirit (and security considerations) anyway? Alternatively, a separate option in Settings for extension support seems like a good idea too.

SpacingBat3 commented 9 months ago

(...) since it's so similar to DevTools in spirit (and security considerations) anyway?

Not really, injecting scripts in DevTools still require manual interaction from the user. Adding a user-writtable path however makes it also possible for third-party scripts and software to mess around with WebCord – while some could do it in legitimate way, others could use that to inject malicious scripts, without any indication for the user.

And CSS themes are kinda protected against that… except in this case, CSS themes aren't huge threat, I think the worst you can do is to add a circular reference in the CSS themes in order to make WebCord recurse for infinity; there would be of course a way to prevent that, I had some plans to take a look on it. But any kind of JS executing without user consent is a bit problematic since third parties can gain much greater control over the app this way.

At least, that is a rationale why extensions support isn't just added to the client. On the other hand, the one could also mess around with app.asar or even the Electron executable if it isn't read-only… and with most Linux packaging designs, it is actually protected that way. I think I could then allow loading extensions the same way – using the directory, let it be in app.asar or nearby it.

czkz commented 9 months ago

I can't think of a scenario/threat where this security measure would make a difference.

XSS from Discord user content can't access the filesystem (to my knowledge), so it's not a problem. Third-party sandboxed malware can't access files outside it's own sandbox, so it's also not a problem. And once you have unsandboxed malware, you can't trust pretty much anything you see - your sudo binaries could be aliased, your display servers - proxied, your kernels - patched. I don't think WebCord can protect you against that :)

I see that flatpak (Flatseal) allows exporting environment variables - maybe those could be used - since access to them allows to forge arbitrary user input anyway?

SpacingBat3 commented 9 months ago

@czkz there are APIs in Chrome extensions (I have no clue tho which are supported, since Electron has its own extension implementation) that allow for it to do more than usual JS scripts within the webpage. Plus, count that extensions could load before Discord does some work on removing the access to the token is enough for me to decline the idea to consider Chrome extensions "safe" enough. Also this reason:

Not really, injecting scripts in DevTools still require manual interaction from the user. Adding a user-writtable path however makes it also possible for third-party scripts and software to mess around with WebCord – while some could do it in legitimate way, others could use that to inject malicious scripts, without any indication for the user.

With extension API being kinda questionable in Electron, I wouldn't risk of some people taking advantage of it to mess around with Discord's data or maybe even going further if there would be some vulnerability around the Electron's implementation of extensions that is not the case for Chrome/Chromium.

And once you have unsandboxed malware, you can't trust pretty much anything you see - your sudo binaries could be aliased, your display servers - proxied, your kernels - patched.

Not without root/admin access in the first place. If any user could just modify any file within the system, well... Just imagine how simple remote code execution could get dangerous each time it happen, and guess what - your browser probably executes some code based on the remote data right now. I mean, vulnerabilities are common thing and I wouldn't be as trustworthy with any browsers to run them in context that allows them to do sh*t to the whole OS (i.e. I don't run any browser as root or admin user lol), and even in some cases – way beyond that.

I see that flatpak (Flatseal) allows exporting environment variables - maybe those could be used - since access to them allows to forge arbitrary user input anyway?

WebCord is not just about flatpaks, not everyone uses that or wants to. Every time I hear about them explained as some superior packaging solution or just mentioned like it is the single way to install software or packaging format makes me think some people are just ignorant about other (some just being awesome and reason why I've chosen a specific distro over the another) packages, and I kinda feel like they are also ignorant about the key difference between Linux distros – package managers.