From a48d81d4cbafc5763f66b78a9863fd3a3b13fca1 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Tue, 3 Sep 2019 17:01:33 +0000 Subject: [PATCH] Bug 1577706 - move checks for -url from toolkit into browser code, and make osint sanitizer app-agnostic, r=mossop Differential Revision: https://phabricator.services.mozilla.com/D44395 --HG-- extra : moz-landing-system : lando --- browser/app/nsBrowserApp.cpp | 5 ++ .../app/winlauncher/LauncherProcessWin.cpp | 4 +- toolkit/xre/CmdLineAndEnvUtils.h | 50 ++++++++++++------- toolkit/xre/nsAppRunner.cpp | 3 -- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/browser/app/nsBrowserApp.cpp b/browser/app/nsBrowserApp.cpp index ac580de403b7..374853c07525 100644 --- a/browser/app/nsBrowserApp.cpp +++ b/browser/app/nsBrowserApp.cpp @@ -4,6 +4,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "nsXULAppAPI.h" +#include "mozilla/CmdLineAndEnvUtils.h" #include "mozilla/XREAppData.h" #include "application.ini.h" #include "mozilla/Bootstrap.h" @@ -210,6 +211,10 @@ static int do_main(int argc, char* argv[], char* envp[]) { gBootstrap->XRE_LibFuzzerSetDriver(fuzzer::FuzzerDriver); #endif + // Note: keep in sync with LauncherProcessWin. + const char* acceptableParams[] = {"url", nullptr}; + EnsureCommandlineSafe(argc, argv, acceptableParams); + return gBootstrap->XRE_main(argc, argv, config); } diff --git a/browser/app/winlauncher/LauncherProcessWin.cpp b/browser/app/winlauncher/LauncherProcessWin.cpp index c9daa9b0ff38..d30b9774033e 100644 --- a/browser/app/winlauncher/LauncherProcessWin.cpp +++ b/browser/app/winlauncher/LauncherProcessWin.cpp @@ -213,7 +213,9 @@ namespace mozilla { Maybe LauncherMain(int& argc, wchar_t* argv[], const StaticXREAppData& aAppData) { - EnsureCommandlineSafe(argc, argv); + // Note: keep in sync with nsBrowserApp. + const wchar_t* acceptableParams[] = {L"url", nullptr}; + EnsureCommandlineSafe(argc, argv, acceptableParams); SetLauncherErrorAppData(aAppData); diff --git a/toolkit/xre/CmdLineAndEnvUtils.h b/toolkit/xre/CmdLineAndEnvUtils.h index e21a03454cda..98ff001cc98a 100644 --- a/toolkit/xre/CmdLineAndEnvUtils.h +++ b/toolkit/xre/CmdLineAndEnvUtils.h @@ -88,7 +88,7 @@ inline bool strimatch(const wchar_t* lowerstr, const wchar_t* mixedstr) { return internal::strimatch(&towlower, lowerstr, mixedstr); } -enum class FlagLiteral { osint, safemode, url }; +enum class FlagLiteral { osint, safemode }; template inline const CharT* GetLiteral(); @@ -106,7 +106,6 @@ inline const CharT* GetLiteral(); DECLARE_FLAG_LITERAL(osint, "osint") DECLARE_FLAG_LITERAL(safemode, "safe-mode") -DECLARE_FLAG_LITERAL(url, "url") enum class CheckArgFlag : uint32_t { None = 0, @@ -193,9 +192,12 @@ inline ArgResult CheckArg(int& aArgc, CharT** aArgv, const CharT* aArg, } template -inline void EnsureCommandlineSafe(int& aArgc, CharT** aArgv) { +inline void EnsureCommandlineSafe(int& aArgc, CharT** aArgv, + const CharT** aAcceptableArgs) { // We expect either no -osint, or the full commandline to be: - // app -osint -url foo + // app -osint + // followed by one of the arguments listed in aAcceptableArgs, + // followed by one parameter for that arg. // If this varies, we abort to avoid abuse of other commandline handlers // from apps that do a poor job escaping links they give to the OS. @@ -203,7 +205,8 @@ inline void EnsureCommandlineSafe(int& aArgc, CharT** aArgv) { if (CheckArg(aArgc, aArgv, osintLit, static_cast(nullptr), CheckArgFlag::None) == ARG_FOUND) { - // There should be 4 items left (app name + -osint + -url + param) + // There should be 4 items left (app name + -osint + (acceptable arg) + + // param) if (aArgc != 4) { exit(127); } @@ -227,7 +230,7 @@ inline void EnsureCommandlineSafe(int& aArgc, CharT** aArgv) { // Strip it: RemoveArg(aArgc, aArgv + 1); - // Now only the -url bit should be left: + // Now only an acceptable argument and a parameter for it should be left: arg = aArgv[1]; if (*arg != '-' #ifdef XP_WIN @@ -240,10 +243,19 @@ inline void EnsureCommandlineSafe(int& aArgc, CharT** aArgv) { if (*arg == '-') { ++arg; } - if (!strimatch(GetLiteral(), arg)) { + bool haveAcceptableArg = false; + const CharT** acceptableArg = aAcceptableArgs; + while (*acceptableArg) { + if (strimatch(*acceptableArg, arg)) { + haveAcceptableArg = true; + break; + } + acceptableArg++; + } + if (!haveAcceptableArg) { exit(127); } - // The param after url shouldn't be another switch: + // The param that is passed afterwards shouldn't be another switch: arg = aArgv[2]; if (*arg == '-' #ifdef XP_WIN @@ -432,9 +444,13 @@ inline bool SetArgv0ToFullBinaryPath(wchar_t* aArgv[]) { #endif // defined(XP_WIN) +// SaveToEnv and EnvHasValue are only available on Windows or when +// MOZILLA_INTERNAL_API is defined +#if defined(MOZILLA_INTERNAL_API) || defined(XP_WIN) + // Save literal putenv string to environment variable. MOZ_NEVER_INLINE inline void SaveToEnv(const char* aEnvString) { -#if defined(MOZILLA_INTERNAL_API) +# if defined(MOZILLA_INTERNAL_API) char* expr = strdup(aEnvString); if (expr) { PR_SetEnv(expr); @@ -442,29 +458,27 @@ MOZ_NEVER_INLINE inline void SaveToEnv(const char* aEnvString) { // We intentionally leak |expr| here since it is required by PR_SetEnv. MOZ_LSAN_INTENTIONALLY_LEAK_OBJECT(expr); -#elif defined(XP_WIN) +# elif defined(XP_WIN) // This is the same as the NSPR implementation // (Note that we don't need to do a strdup for this case; the CRT makes a // copy) _putenv(aEnvString); -#else -# error "Not implemented for this configuration" -#endif +# endif } inline bool EnvHasValue(const char* aVarName) { -#if defined(MOZILLA_INTERNAL_API) +# if defined(MOZILLA_INTERNAL_API) const char* val = PR_GetEnv(aVarName); return val && *val; -#elif defined(XP_WIN) +# elif defined(XP_WIN) // This is the same as the NSPR implementation const char* val = getenv(aVarName); return val && *val; -#else -# error "Not implemented for this configuration" -#endif +# endif } +#endif // end windows/internal_api-only definitions + #ifndef NS_NO_XPCOM already_AddRefed GetFileFromEnv(const char* name); #endif diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 80b926d71367..956c1873fc6d 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -4599,9 +4599,6 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) { gArgc = argc; gArgv = argv; - EnsureCommandlineSafe(gArgc, gArgv); - // DO NOT TOUCH THE COMMANDLINE ARGS BEFORE THIS! - ScopedLogging log; mozilla::LogModule::Init(gArgc, gArgv);