Skip to content

Commit

Permalink
fix: only remove hijackable envs from foreign parent (#41099)
Browse files Browse the repository at this point in the history
Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
  • Loading branch information
trop[bot] and zcbenz committed Jan 26, 2024
1 parent 682b27f commit e16d9f9
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 32 deletions.
1 change: 0 additions & 1 deletion filenames.gni
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,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
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,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 @@ -123,8 +140,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
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ v8::MaybeLocal<v8::Value> CompileAndCall(
std::vector<v8::Local<v8::Value>>* arguments,
node::Environment* optional_env);

#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.

0 comments on commit e16d9f9

Please sign in to comment.