Browse Source

fix: only remove hijackable envs from foreign parent (#41102)

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Cheng Zhao <[email protected]>
trop[bot] 1 year ago
parent
commit
0fc5e42018
4 changed files with 19 additions and 32 deletions
  1. 0 1
      filenames.gni
  2. 19 2
      shell/app/node_main.cc
  3. 0 6
      shell/common/node_util.h
  4. 0 23
      shell/common/node_util_mac.mm

+ 0 - 1
filenames.gni

@@ -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",

+ 19 - 2
shell/app/node_main.cc

@@ -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) {}
@@ -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.";
     }

+ 0 - 6
shell/common/node_util.h

@@ -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_

+ 0 - 23
shell/common/node_util_mac.mm

@@ -1,23 +0,0 @@
-// Copyright (c) 2023 Microsoft, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#include "shell/common/node_util.h"
-
-#include <Foundation/Foundation.h>
-
-namespace electron::util {
-
-bool UnsetAllNodeEnvs() {
-  bool has_unset = false;
-  for (NSString* env in NSProcessInfo.processInfo.environment) {
-    if (![env hasPrefix:@"NODE_"])
-      continue;
-    const char* name = [[env componentsSeparatedByString:@"="][0] UTF8String];
-    unsetenv(name);
-    has_unset = true;
-  }
-  return has_unset;
-}
-
-}  // namespace electron::util