EnhancedDiscord icon indicating copy to clipboard operation
EnhancedDiscord copied to clipboard

Improper Electron Security Practices

Open night opened this issue 3 years ago • 3 comments

Upon reviewing this project's "injector" code, it appears it disables numerous security features implemented by Discord to ensure remote code is sufficiently sandboxed from the operating system. As it stands, this software is a walking remote code execution waiting to happen.

  1. Node Integration Enabled
        originalOptions.webPreferences.nodeIntegration = true;

This software leaks node integration into the main window. This means the window has access to directly modify the file system and execute arbitrary commands.

  1. Remote Module Enabled
        originalOptions.webPreferences.enableRemoteModule = true;

This software enables Electron's remote module in the main window. This means the window has access to send direct IPC commands which can be used to execute arbitrary code. The remote module is also being removed in the next version of Electron, so you will have to fix this anyways when that occurs.

  1. Context Isolation Disabled
        originalOptions.webPreferences.contextIsolation = false;

This software disables Electron's context isolation, which forces browser code to run in a separate context from main window code. This prevents attackers from doing things like polluting prototypes which may expose access to restricted functions that escalate access to execute arbitrary commands.

  1. Content Security Policy (CSP) Disabled
electron.session.defaultSession.webRequest.onHeadersReceived(function(details, callback) {
    if (!details.responseHeaders['content-security-policy-report-only'] && !details.responseHeaders['content-security-policy']) return callback({cancel: false});
    delete details.responseHeaders['content-security-policy-report-only'];
    delete details.responseHeaders['content-security-policy'];
    callback({cancel: false, responseHeaders: details.responseHeaders});
});

CSP exists to mitigate and prevent attacks around most XSS and content injection. If someone finds XSS in Discord, the lack of 1, 2, and 3 listed above would directly result in remote code execution.

Security of Electron is not to be taken lightly as there are many foot-guns. By releasing software like this and encouraging people to install it, you are putting users at risk without taking proper steps to keep Electron secure. I would strongly encourage you to read up on the best security practices for Electron at https://www.electronjs.org/docs/tutorial/security and apply those to this project.

night avatar Sep 07 '20 09:09 night

This may all be true, but what other choice do we have? If Discord cared to make an easy way for client mods to inject, or just plugin/theme support in general, this wouldn't be a problem. But by fighting client mods, they give users the choice to allow the Discord app to have a fraction of its usefulness or have reduced security. Not to mention client mods have to use the same injection methods as malware, which hurts our reputation and blurs the line between good and bad Discord modifications. I can't tell y'all what to do, but if you really care about client mod users' security, you could just add official support for some of the things we've had to awkwardly patch in for years.

joe27g avatar Sep 07 '20 12:09 joe27g

Note: I don't mean to be hostile to you (night) or any other Discord employees, I was just trying to point out that it's a bit ironic to give suggestions to projects that you generally don't support/advocate against using. I just assumed that the reason for creating this issue was to call out client mods as being insecure, but I understand now you were just trying to help. No hard feelings.

Anyway, as for the issues themselves, I'm not interested in fixing them for a few reasons:

  • These are mostly carried over from BD, so I don't remember why exactly each of these options were changed.
  • The CSP is probably the most likely to be abused, so I'd love to just re-enable it and prevent any non-Discord remote scripts from loading, but that would break themes and many BD plugins.
  • Reverting these changes or implementing alternative solutions would require testing with many different plugins, including BD plugins. I don't have time to figure out what the alternatives are or do said testing, hence the "help wanted" label.

For reference, https://github.com/powercord-org/powercord/issues/386 has more info and discussion about these issues.

joe27g avatar Sep 08 '20 22:09 joe27g

Accounted for and partially fixed by last update, EnhancedDiscord has cut off updates, and all remaining official support will terminate April 12th.

MasicoreLord avatar Mar 16 '21 05:03 MasicoreLord