Skip to content
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

Security: Don't run the options as JS with access to some powerful objects #8

Open
Fighter178 opened this issue Apr 6, 2024 · 1 comment

Comments

@Fighter178
Copy link

Fighter178 commented Apr 6, 2024

The options are ran as pure JS, with as far as I can tell, very little to no sandboxing (I haven't dug through the code to find out for sure).
Powerful objects can be used by adding code to the options. While JS allows for a lot of flexibility, allowing to be ran without any checks is very dangerous.

The simplest solution to improve security is to do this:

const optionsRaw = getSavedOptions(); // Or however you get the options
const argNames = .["`isSecureContext", "chrome", ...Object.keys(window)]
const parseOptionsFunctionRaw = new Function(..argNames, `${optionsRaw};return FS_OPTIONS`);
// Predefine a few arguments, but be sure to add them to the function argument array too.
const predefinedArgs = [window.isSecureContext, null];
const args=  predefinedArgs.fill(predefinedArgs.length, paseOptionsFunctionRaw.length, null);
const parseOptions = ()=>parseOpptionsRaw.call(null, ...args);
const options = parseOptions();

This makes hacking harder but doesn't make it impossible. It makes simple attacks much more difficult. The best solution is to prevent using JS at all. Perhaps have drop-downs for some common options for things that are true/false based on condition and then allow inputs that don't run as JS for flexible options.

Doing it this way helps make sure that it is harder to gain access with simple attacks, but running any JS at all, especially not in any sort of sandbox is still dangerous. A better solution would be to use a web worker, but I don't know how to implement this is a browser extension.

Also make a SECURITY.md file so people know how to report issues like this instead of creating a GitHub issue.

This issue requires someone with access to the user's browser, either locally or with some remote access program, and likely one that supports a GUI.

@ichaoX
Copy link
Owner

ichaoX commented Apr 7, 2024

The options are ran as pure JS, with as far as I can tell, very little to no sandboxing (I haven't dug through the code to find out for sure). Powerful objects can be used by adding code to the options. While JS allows for a lot of flexibility, allowing to be ran without any checks is very dangerous.

Content scripts run in an isolated sandbox, at least separated from page scripts.

Your implication is that someone might save dangerous JavaScript code in "Content Script > Configuration parameters," right? In that case, extensions like Tampermonkey are equally risky.

The best solution is to prevent using JS at all.

It will be a trade-off between flexibility, security, and development complexity.
Additionally, new Function involves unsafe-eval, which may lead to additional Content Security Policy issues.

Also make a SECURITY.md file so people know how to report issues like this instead of creating a GitHub issue.

Now you should be able to use the GitHub Security Advisory "Report a Vulnerability" tab.

This issue requires someone with access to the user's browser, either locally or with some remote access program, and likely one that supports a GUI.

Adding a security warning might reduce the likelihood of user errors.
If the browser is accessed without authorization, checking or restoring settings can prevent attacks from persisting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants