fix: -Wunsafe-buffer-usage warnings with argc, argv (#44366)

* refactor: move uv_setup_args() calls to startup

* refactor: call base::CommandLine::Init() before ContentMain()

* feat: add ElectronCommandLine::AsUtf8()

* refactor: call base::CommandLine::Init() before NodeMain()

* refactor: use ElectronCommandLine::AsUtf8() in NodeMain()

* fix: -Wunsafe-buffer-usage warning in ElectronCommandLine::Init()

* chore: add a DCHECK to confirm ElectronCommandLine was initialized before AsUtf8() is called

* chore: const correctness in ElectronCommandLine::Init() args

* chore: add ElectronCommandLine to macOS Electron Helper app

* chore: move argc, argvc setup into electron_library_main on macOS

* chore: revert BUILD.gn changes

* fix: WideToUTF8() call in ElectronCommandLine::AsUtf8()

* build: add uv to the include paths for app/electron_main_linux

* build: add uv to the include paths for app/electron_library_main.mm

* chore: revert unrelated changes

these were intended for another branch

BUILD.gn

@@ -865,6 +865,7 @@ if (is_mac) {
       ":electron_framework_resources",
       ":electron_swiftshader_library",
       ":electron_xibs",
+      "//third_party/electron_node:node_lib",
     ]
     if (!is_mas_build) {
       deps += [ ":electron_crashpad_helper" ]
@@ -1188,6 +1189,7 @@ if (is_mac) {
       "//components/crash/core/app",
       "//content:sandbox_helper_win",
       "//electron/buildflags",
+      "//third_party/electron_node:node_lib",
       "//ui/strings",
     ]
 

shell/app/electron_library_main.mm

@@ -9,6 +9,7 @@
 #include "base/apple/bundle_locations.h"
 #include "base/apple/scoped_nsautorelease_pool.h"
 #include "base/at_exit.h"
+#include "base/command_line.h"
 #include "base/i18n/icu_util.h"
 #include "content/public/app/content_main.h"
 #include "electron/fuses.h"
@@ -16,21 +17,22 @@
 #include "shell/app/node_main.h"
 #include "shell/common/electron_command_line.h"
 #include "shell/common/mac/main_application_bundle.h"
+#include "uv.h"
 
 int ElectronMain(int argc, char* argv[]) {
-  electron::ElectronMainDelegate delegate;
-  content::ContentMainParams params(&delegate);
-  params.argc = argc;
-  params.argv = const_cast<const char**>(argv);
+  argv = uv_setup_args(argc, argv);
+  base::CommandLine::Init(argc, argv);
   electron::ElectronCommandLine::Init(argc, argv);
 
+  electron::ElectronMainDelegate delegate;
+
   // Ensure that Bundle Id is set before ContentMain.
   // Refs https://chromium-review.googlesource.com/c/chromium/src/+/5581006
   delegate.OverrideChildProcessPath();
   delegate.OverrideFrameworkBundlePath();
   delegate.SetUpBundleOverrides();
 
-  return content::ContentMain(std::move(params));
+  return content::ContentMain(content::ContentMainParams{&delegate});
 }
 
 int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
@@ -39,6 +41,10 @@ int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
     return 1;
   }
 
+  argv = uv_setup_args(argc, argv);
+  base::CommandLine::Init(argc, argv);
+  electron::ElectronCommandLine::Init(argc, argv);
+
   base::AtExitManager atexit_manager;
   base::apple::ScopedNSAutoreleasePool pool;
   base::apple::SetOverrideFrameworkBundlePath(
@@ -47,5 +53,5 @@ int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
           .Append("Frameworks")
           .Append(ELECTRON_PRODUCT_NAME " Framework.framework"));
   base::i18n::InitializeICU();
-  return electron::NodeMain(argc, argv);
+  return electron::NodeMain();
 }

shell/app/electron_main_linux.cc

@@ -16,6 +16,7 @@
 #include "shell/app/uv_stdio_fix.h"
 #include "shell/common/electron_command_line.h"
 #include "shell/common/electron_constants.h"
+#include "uv.h"
 
 namespace {
 
@@ -29,17 +30,16 @@ namespace {
 int main(int argc, char* argv[]) {
   FixStdioStreams();
 
+  argv = uv_setup_args(argc, argv);
+  base::CommandLine::Init(argc, argv);
+  electron::ElectronCommandLine::Init(argc, argv);
+
   if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
     base::i18n::InitializeICU();
     base::AtExitManager atexit_manager;
-    return electron::NodeMain(argc, argv);
+    return electron::NodeMain();
   }
 
   electron::ElectronMainDelegate delegate;
-  content::ContentMainParams params(&delegate);
-  electron::ElectronCommandLine::Init(argc, argv);
-  params.argc = argc;
-  params.argv = const_cast<const char**>(argv);
-  base::CommandLine::Init(params.argc, params.argv);
-  return content::ContentMain(std::move(params));
+  return content::ContentMain(content::ContentMainParams{&delegate});
 }

shell/app/electron_main_win.cc

@@ -127,16 +127,15 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
   // If we are already a fiber then continue normal execution.
 #endif  // defined(ARCH_CPU_32_BITS)
 
-  struct Arguments {
+  {
     int argc = 0;
-    RAW_PTR_EXCLUSION wchar_t** argv =
-        ::CommandLineToArgvW(::GetCommandLineW(), &argc);
-
-    ~Arguments() { LocalFree(argv); }
-  } arguments;
-
-  if (!arguments.argv)
-    return -1;
+    wchar_t** argv = ::CommandLineToArgvW(::GetCommandLineW(), &argc);
+    if (!argv)
+      return -1;
+    base::CommandLine::Init(0, nullptr);  // args ignored on Windows
+    electron::ElectronCommandLine::Init(argc, argv);
+    LocalFree(argv);
+  }
 
 #ifdef _DEBUG
   // Don't display assert dialog boxes in CI test runs
@@ -159,18 +158,12 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
   if (run_as_node || !IsEnvSet("ELECTRON_NO_ATTACH_CONSOLE"))
     base::RouteStdioToConsole(false);
 
-  std::vector<char*> argv(arguments.argc);
-  std::transform(arguments.argv, arguments.argv + arguments.argc, argv.begin(),
-                 [](auto& a) { return _strdup(base::WideToUTF8(a).c_str()); });
   if (run_as_node) {
     base::AtExitManager atexit_manager;
     base::i18n::InitializeICU();
-    auto ret = electron::NodeMain(argv.size(), argv.data());
-    std::ranges::for_each(argv, free);
-    return ret;
+    return electron::NodeMain();
   }
 
-  base::CommandLine::Init(argv.size(), argv.data());
   const base::CommandLine* command_line =
       base::CommandLine::ForCurrentProcess();
 
@@ -235,6 +228,5 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
   content::ContentMainParams params(&delegate);
   params.instance = instance;
   params.sandbox_info = &sandbox_info;
-  electron::ElectronCommandLine::Init(arguments.argc, arguments.argv);
   return content::ContentMain(std::move(params));
 }

shell/app/node_main.cc

@@ -27,6 +27,7 @@
 #include "shell/app/uv_task_runner.h"
 #include "shell/browser/javascript_environment.h"
 #include "shell/common/api/electron_bindings.h"
+#include "shell/common/electron_command_line.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
@@ -115,12 +116,8 @@ v8::Local<v8::Value> GetParameters(v8::Isolate* isolate) {
 
 namespace electron {
 
-int NodeMain(int argc, char* argv[]) {
-  bool initialized = base::CommandLine::Init(argc, argv);
-  if (!initialized) {
-    LOG(ERROR) << "Failed to initialize CommandLine";
-    exit(1);
-  }
+int NodeMain() {
+  DCHECK(base::CommandLine::InitializedForCurrentProcess());
 
   auto os_env = base::Environment::Create();
   bool node_options_enabled = electron::fuses::IsNodeOptionsEnabled();
@@ -182,11 +179,8 @@ int NodeMain(int argc, char* argv[]) {
     // Explicitly register electron's builtin bindings.
     NodeBindings::RegisterBuiltinBindings();
 
-    // Hack around with the argv pointer. Used for process.title = "blah".
-    argv = uv_setup_args(argc, argv);
-
     // Parse Node.js cli flags and strip out disallowed options.
-    std::vector<std::string> args(argv, argv + argc);
+    const std::vector<std::string> args = ElectronCommandLine::AsUtf8();
     ExitIfContainsDisallowedFlags(args);
 
     std::unique_ptr<node::InitializationResult> result =

shell/app/node_main.h

@@ -7,7 +7,7 @@
 
 namespace electron {
 
-int NodeMain(int argc, char* argv[]);
+int NodeMain();
 
 }  // namespace electron
 

shell/common/electron_command_line.cc

@@ -5,7 +5,8 @@
 #include "shell/common/electron_command_line.h"
 
 #include "base/command_line.h"
-#include "uv.h"  // NOLINT(build/include_directory)
+#include "base/containers/to_vector.h"
+#include "base/strings/utf_string_conversions.h"
 
 namespace electron {
 
@@ -13,17 +14,25 @@ namespace electron {
 base::CommandLine::StringVector ElectronCommandLine::argv_;
 
 // static
-void ElectronCommandLine::Init(int argc, base::CommandLine::CharType** argv) {
+void ElectronCommandLine::Init(int argc,
+                               base::CommandLine::CharType const* const* argv) {
   DCHECK(argv_.empty());
 
-  // NOTE: uv_setup_args does nothing on Windows, so we don't need to call it.
-  // Otherwise we'd have to convert the arguments from UTF16.
-#if !BUILDFLAG(IS_WIN)
-  // Hack around with the argv pointer. Used for process.title = "blah"
-  argv = uv_setup_args(argc, argv);
-#endif
+  // Safety: as is normal in command lines, argc and argv must correspond
+  // to one another. Otherwise there will be out-of-bounds accesses.
+  argv_.assign(argv, UNSAFE_BUFFERS(argv + argc));
+}
 
-  argv_.assign(argv, argv + argc);
+// static
+std::vector<std::string> ElectronCommandLine::AsUtf8() {
+  DCHECK(!argv_.empty());
+
+#if BUILDFLAG(IS_WIN)
+  return base::ToVector(
+      argv_, [](const auto& wstr) { return base::WideToUTF8(wstr); });
+#else
+  return argv_;
+#endif
 }
 
 #if BUILDFLAG(IS_LINUX)

shell/common/electron_command_line.h

@@ -20,7 +20,9 @@ class ElectronCommandLine {
 
   static const base::CommandLine::StringVector& argv() { return argv_; }
 
-  static void Init(int argc, base::CommandLine::CharType** argv);
+  static std::vector<std::string> AsUtf8();
+
+  static void Init(int argc, base::CommandLine::CharType const* const* argv);
 
 #if BUILDFLAG(IS_LINUX)
   // On Linux the command line has to be read from base::CommandLine since

shell/common/node_bindings.cc

@@ -799,15 +799,8 @@ std::shared_ptr<node::Environment> NodeBindings::CreateEnvironment(
     v8::Local<v8::Context> context,
     node::MultiIsolatePlatform* platform,
     std::optional<base::RepeatingCallback<void()>> on_app_code_ready) {
-#if BUILDFLAG(IS_WIN)
-  auto& electron_args = ElectronCommandLine::argv();
-  std::vector<std::string> args(electron_args.size());
-  std::ranges::transform(electron_args, args.begin(),
-                         [](auto& a) { return base::WideToUTF8(a); });
-#else
-  auto args = ElectronCommandLine::argv();
-#endif
-  return CreateEnvironment(context, platform, args, {}, on_app_code_ready);
+  return CreateEnvironment(context, platform, ElectronCommandLine::AsUtf8(), {},
+                           on_app_code_ready);
 }
 
 void NodeBindings::LoadEnvironment(node::Environment* env) {