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

fix: only remove hijackable envs from foreign parent #41102

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion filenames.gni
Expand Up @@ -660,7 +660,6 @@ filenames = {
"shell/common/node_includes.h",
"shell/common/node_util.cc",
"shell/common/node_util.h",
"shell/common/node_util_mac.mm",
"shell/common/options_switches.cc",
"shell/common/options_switches.h",
"shell/common/platform_util.cc",
Expand Down
21 changes: 19 additions & 2 deletions shell/app/node_main.cc
Expand Up @@ -83,6 +83,23 @@ void ExitIfContainsDisallowedFlags(const std::vector<std::string>& argv) {
}
}

#if BUILDFLAG(IS_MAC)
// A list of node envs that may be used to inject scripts.
const char* kHijackableEnvs[] = {"NODE_OPTIONS", "NODE_REPL_EXTERNAL_MODULE"};

// Return true if there is any env in kHijackableEnvs.
bool UnsetHijackableEnvs(base::Environment* env) {
bool has = false;
for (const char* name : kHijackableEnvs) {
if (env->HasVar(name)) {
env->UnSetVar(name);
has = true;
}
}
return has;
}
#endif

#if IS_MAS_BUILD()
void SetCrashKeyStub(const std::string& key, const std::string& value) {}
void ClearCrashKeyStub(const std::string& key) {}
Expand Down Expand Up @@ -124,8 +141,8 @@ int NodeMain(int argc, char* argv[]) {
// NODE_OPTIONS: "--require 'bad.js'"}})
// To prevent Electron apps from being used to work around macOS security
// restrictions, when the parent process is not part of the app bundle, all
// environment variables starting with NODE_ will be removed.
if (util::UnsetAllNodeEnvs()) {
// environment variables that may be used to inject scripts are removed.
if (UnsetHijackableEnvs(os_env.get())) {
LOG(ERROR) << "Node.js environment variables are disabled because this "
"process is invoked by other apps.";
}
Expand Down
6 changes: 0 additions & 6 deletions shell/common/node_util.h
Expand Up @@ -27,12 +27,6 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
std::vector<v8::Local<v8::String>>* parameters,
std::vector<v8::Local<v8::Value>>* arguments);

#if BUILDFLAG(IS_MAC)
// Unset all environment variables that start with NODE_. Return false if there
// is no node env at all.
bool UnsetAllNodeEnvs();
#endif

} // namespace electron::util

#endif // ELECTRON_SHELL_COMMON_NODE_UTIL_H_
23 changes: 0 additions & 23 deletions shell/common/node_util_mac.mm

This file was deleted.