diff --git a/shell/app/command_line_args.cc b/shell/app/command_line_args.cc index b39e242c0146..87250dd24f30 100644 --- a/shell/app/command_line_args.cc +++ b/shell/app/command_line_args.cc @@ -4,6 +4,7 @@ #include "shell/app/command_line_args.h" +#include #include #include "sandbox/policy/switches.h" @@ -11,46 +12,44 @@ 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; +#if BUILDFLAG(IS_WIN) +constexpr auto DashDash = base::CommandLine::StringPieceType{L"--"}; +#else +constexpr auto DashDash = base::CommandLine::StringPieceType{"--"}; +#endif - // 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; +// 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::StringPieceType arg) { + const auto scheme_end = arg.find(':'); + if (scheme_end == base::CommandLine::StringPieceType::npos) + return false; - return true; - } - - // white-space before a colon means it's not a URL - if (std::isspace(c, std::locale::classic())) - break; - } - } - - 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; } diff --git a/shell/app/command_line_args.h b/shell/app/command_line_args.h index 70e1e6b0579b..6a8c78b4a958 100644 --- a/shell/app/command_line_args.h +++ b/shell/app/command_line_args.h @@ -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 diff --git a/shell/app/electron_main_win.cc b/shell/app/electron_main_win.cc index e78efc18a5b4..b6c673652e80 100644 --- a/shell/app/electron_main_win.cc +++ b/shell/app/electron_main_win.cc @@ -225,7 +225,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};