Repository: electron/electron
Pull request: fix: -Wunsafe-buffer-usage warnings in IsUrlArg()

#### Description of Change

Upstream is starting to use `-Wunsafe-buffer-usage` as part of a [larger C buffer hardening project at Google](https://bughunters.google.com/blog/6368559657254912/llvm-s-rfc-c-buffer-hardening-at-google) and [in Chromium](https://issues.chromium.org/issues/40285824). We should be doing this, too. See also [this thread "Introducing Spanification"](https://groups.google.com/a/chromium.org/g/chromium-dev/c/iEy69ygz-rs) .

This is part 1 in a small series to fix `-Wunsafe-buffer-usage` warnings in Electron. My intent is to fix all of them and then add the clang warning to our cflags.

We have 44 such warnings in `shell/`. This PR fixes the first three :smile_cat: 

```sh
46362:../../electron/shell/app/command_line_args.cc:18:20: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
46365:../../electron/shell/app/command_line_args.cc:18:20: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
46366:../../electron/shell/app/command_line_args.cc:18:35: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
46369:../../electron/shell/app/command_line_args.cc:18:35: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
46370:../../electron/shell/app/command_line_args.cc:24:18: warning: unsafe pointer arithmetic [-Wunsafe-buffer-usage]
46373:../../electron/shell/app/command_line_args.cc:24:18: note: pass -fsafe-buffer-usage-suggestions to receive code hardening suggestions
```

#### Checklist

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: none.

Changed files:
filename: shell/app/command_line_args.cc
patch: @@ -4,53 +4,52 @@
 
 #include "shell/app/command_line_args.h"
 
+#include <algorithm>
 #include <locale>
 
 #include "sandbox/policy/switches.h"
 #include "shell/common/options_switches.h"
 
 namespace {
 
-bool IsUrlArg(const base::CommandLine::CharType* arg) {
-  // the first character must be a letter for this to be a URL
-  auto c = *arg;
-  if (std::isalpha(c, std::locale::classic())) {
-    for (auto* p = arg + 1; *p; ++p) {
-      c = *p;
-
-      // colon indicates that the argument starts with a URI scheme
-      if (c == ':') {
-        // it could also be a Windows filesystem path
-        if (p == arg + 1)
-          break;
-
-        return true;
-      }
-
-      // white-space before a colon means it's not a URL
-      if (std::isspace(c, std::locale::classic()))
-        break;
-    }
-  }
-
-  return false;
+#if BUILDFLAG(IS_WIN)
+constexpr auto DashDash = base::CommandLine::StringViewType{L"--"};
+#else
+constexpr auto DashDash = base::CommandLine::StringViewType{"--"};
+#endif
+
+// we say it's a URL arg if it starts with a URI scheme that:
+// 1. starts with an alpha, and
+// 2. contains no spaces, and
+// 3. is longer than one char (to ensure it's not a Windows drive path)
+bool IsUrlArg(const base::CommandLine::StringViewType arg) {
+  const auto scheme_end = arg.find(':');
+  if (scheme_end == base::CommandLine::StringViewType::npos)
+    return false;
+
+  const auto& c_locale = std::locale::classic();
+  const auto isspace = [&](auto ch) { return std::isspace(ch, c_locale); };
+  const auto scheme = arg.substr(0U, scheme_end);
+  return std::size(scheme) > 1U && std::isalpha(scheme.front(), c_locale) &&
+         std::ranges::none_of(scheme, isspace);
 }
-
 }  // namespace
 
 namespace electron {
 
-bool CheckCommandLineArguments(int argc, base::CommandLine::CharType** argv) {
-  const base::CommandLine::StringType dashdash(2, '-');
+// Check for CVE-2018-1000006 issues. Return true iff argv looks safe.
+// Sample exploit: 'exodus://aaaaaaaaa" --gpu-launcher="cmd" --aaaaa='
+// Prevent it by returning false if any arg except '--' follows a URL arg.
+// More info at https://www.electronjs.org/blog/protocol-handler-fix
+bool CheckCommandLineArguments(const base::CommandLine::StringVector& argv) {
   bool block_args = false;
-  for (int i = 0; i < argc; ++i) {
-    if (argv[i] == dashdash)
+  for (const auto& arg : argv) {
+    if (arg == DashDash)
       break;
-    if (block_args) {
+    if (block_args)
       return false;
-    } else if (IsUrlArg(argv[i])) {
+    if (IsUrlArg(arg))
       block_args = true;
-    }
   }
   return true;
 }

filename: shell/app/command_line_args.h
patch: @@ -9,7 +9,7 @@
 
 namespace electron {
 
-bool CheckCommandLineArguments(int argc, base::CommandLine::CharType** argv);
+bool CheckCommandLineArguments(const base::CommandLine::StringVector& argv);
 bool IsSandboxEnabled(base::CommandLine* command_line);
 
 }  // namespace electron

filename: shell/app/electron_main_win.cc
patch: @@ -224,7 +224,7 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
   CHECK_EQ(fiber_status, FiberStatus::kSuccess);
 #endif  // defined(ARCH_CPU_32_BITS)
 
-  if (!electron::CheckCommandLineArguments(arguments.argc, arguments.argv))
+  if (!electron::CheckCommandLineArguments(command_line->argv()))
     return -1;
 
   sandbox::SandboxInterfaceInfo sandbox_info = {nullptr};