From 16a4871cdf1f4882f6db01d8352879151fc90158 Mon Sep 17 00:00:00 2001 From: Bob Owen Date: Mon, 22 May 2017 20:41:28 +0100 Subject: [PATCH] Bug 1339105 Part 3: Move NPAPI windows process sandbox file rules into SandboxBroker. r=jimm This also removes a rule that was added for sandboxing the Java plugin, which we never did and we now only allow Flash anyway. --- dom/plugins/ipc/PluginProcessParent.cpp | 74 ---------------- ipc/glue/GeckoChildProcessHost.cpp | 12 --- ipc/glue/GeckoChildProcessHost.h | 2 - .../win/src/sandboxbroker/sandboxBroker.cpp | 85 +++++++++---------- .../win/src/sandboxbroker/sandboxBroker.h | 2 - 5 files changed, 39 insertions(+), 136 deletions(-) diff --git a/dom/plugins/ipc/PluginProcessParent.cpp b/dom/plugins/ipc/PluginProcessParent.cpp index c4dbbdead867..14d1f50b9dc1 100644 --- a/dom/plugins/ipc/PluginProcessParent.cpp +++ b/dom/plugins/ipc/PluginProcessParent.cpp @@ -14,10 +14,6 @@ #include "mozilla/Telemetry.h" #include "nsThreadUtils.h" -#if defined(XP_WIN) && defined(MOZ_SANDBOX) -#include "nsDirectoryServiceDefs.h" -#endif - using std::vector; using std::string; @@ -56,82 +52,12 @@ PluginProcessParent::~PluginProcessParent() #endif } -#if defined(XP_WIN) && defined(MOZ_SANDBOX) -static void -AddSandboxAllowedFile(vector& aAllowedFiles, nsIProperties* aDirSvc, - const char* aDir, const nsAString& aSuffix = EmptyString()) -{ - nsCOMPtr userDir; - nsresult rv = aDirSvc->Get(aDir, NS_GET_IID(nsIFile), getter_AddRefs(userDir)); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - - nsAutoString userDirPath; - rv = userDir->GetPath(userDirPath); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - - if (!aSuffix.IsEmpty()) { - userDirPath.Append(aSuffix); - } - aAllowedFiles.push_back(std::wstring(userDirPath.get())); - return; -} - -static void -AddSandboxAllowedFiles(int32_t aSandboxLevel, - vector& aAllowedFilesRead, - vector& aAllowedFilesReadWrite, - vector& aAllowedDirectories) -{ - if (aSandboxLevel < 2) { - return; - } - - nsresult rv; - nsCOMPtr dirSvc = - do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - - // Level 2 and above is now using low integrity, so we need to give write - // access to the Flash directories. - // This should be made Flash specific (Bug 1171396). - AddSandboxAllowedFile(aAllowedFilesReadWrite, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia\\Flash Player\\*")); - AddSandboxAllowedFile(aAllowedFilesReadWrite, dirSvc, NS_WIN_LOCAL_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia\\Flash Player\\*")); - AddSandboxAllowedFile(aAllowedFilesReadWrite, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Adobe\\Flash Player\\*")); - - // Access also has to be given to create the parent directories as they may - // not exist. - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia")); - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia\\Flash Player")); - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia")); - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_LOCAL_APPDATA_DIR, - NS_LITERAL_STRING("\\Macromedia\\Flash Player")); - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Adobe")); - AddSandboxAllowedFile(aAllowedDirectories, dirSvc, NS_WIN_APPDATA_DIR, - NS_LITERAL_STRING("\\Adobe\\Flash Player")); -} -#endif - bool PluginProcessParent::Launch(mozilla::UniquePtr aLaunchCompleteTask, int32_t aSandboxLevel) { #if defined(XP_WIN) && defined(MOZ_SANDBOX) mSandboxLevel = aSandboxLevel; - AddSandboxAllowedFiles(mSandboxLevel, mAllowedFilesRead, - mAllowedFilesReadWrite, mAllowedDirectories); #else if (aSandboxLevel != 0) { MOZ_ASSERT(false, diff --git a/ipc/glue/GeckoChildProcessHost.cpp b/ipc/glue/GeckoChildProcessHost.cpp index 25bdc5394ba2..20836c7571a1 100644 --- a/ipc/glue/GeckoChildProcessHost.cpp +++ b/ipc/glue/GeckoChildProcessHost.cpp @@ -1028,18 +1028,6 @@ GeckoChildProcessHost::PerformAsyncLaunchInternal(std::vector& aExt ++it) { mSandboxBroker.AllowReadFile(it->c_str()); } - - for (auto it = mAllowedFilesReadWrite.begin(); - it != mAllowedFilesReadWrite.end(); - ++it) { - mSandboxBroker.AllowReadWriteFile(it->c_str()); - } - - for (auto it = mAllowedDirectories.begin(); - it != mAllowedDirectories.end(); - ++it) { - mSandboxBroker.AllowDirectory(it->c_str()); - } } #endif // XP_WIN && MOZ_SANDBOX diff --git a/ipc/glue/GeckoChildProcessHost.h b/ipc/glue/GeckoChildProcessHost.h index ea4e8e207770..8cf76f6019ec 100644 --- a/ipc/glue/GeckoChildProcessHost.h +++ b/ipc/glue/GeckoChildProcessHost.h @@ -157,8 +157,6 @@ protected: #ifdef MOZ_SANDBOX SandboxBroker mSandboxBroker; std::vector mAllowedFilesRead; - std::vector mAllowedFilesReadWrite; - std::vector mAllowedDirectories; bool mEnableSandboxLogging; int32_t mSandboxLevel; #endif diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp index a9d98757acf8..70aad92e8fa2 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ -31,6 +31,8 @@ sandbox::BrokerServices *SandboxBroker::sBrokerService = nullptr; static UniquePtr sBinDir; static UniquePtr sProfileDir; static UniquePtr sContentTempDir; +static UniquePtr sRoamingAppDataDir; +static UniquePtr sLocalAppDataDir; static LazyLogModule sSandboxBrokerLog("SandboxBroker"); @@ -86,6 +88,8 @@ SandboxBroker::CacheRulesDirectories() CacheDirAndAutoClear(dirSvc, NS_GRE_DIR, &sBinDir); CacheDirAndAutoClear(dirSvc, NS_APP_USER_PROFILE_50_DIR, &sProfileDir); CacheDirAndAutoClear(dirSvc, NS_APP_CONTENT_PROCESS_TEMP_DIR, &sContentTempDir); + CacheDirAndAutoClear(dirSvc, NS_WIN_APPDATA_DIR, &sRoamingAppDataDir); + CacheDirAndAutoClear(dirSvc, NS_WIN_LOCAL_APPDATA_DIR, &sLocalAppDataDir); } SandboxBroker::SandboxBroker() @@ -565,6 +569,41 @@ SandboxBroker::SetSecurityLevelForPluginProcess(int32_t aSandboxLevel) SANDBOX_ENSURE_SUCCESS(result, "Invalid flags for SetProcessMitigations."); + if (aSandboxLevel >= 2) { + // Level 2 and above uses low integrity, so we need to give write access to + // the Flash directories. + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Macromedia\\Flash Player\\*")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_ANY, + sLocalAppDataDir, + NS_LITERAL_STRING("\\Macromedia\\Flash Player\\*")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Adobe\\Flash Player\\*")); + + // Access also has to be given to create the parent directories as they may + // not exist. + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Macromedia")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Macromedia\\Flash Player")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sLocalAppDataDir, + NS_LITERAL_STRING("\\Macromedia")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sLocalAppDataDir, + NS_LITERAL_STRING("\\Macromedia\\Flash Player")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Adobe")); + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, + sRoamingAppDataDir, + NS_LITERAL_STRING("\\Adobe\\Flash Player")); + } + // Add the policy for the client side of a pipe. It is just a file // in the \pipe\ namespace. We restrict it to pipes that start with // "chrome." so the sandboxed process cannot connect to system services. @@ -596,13 +635,6 @@ SandboxBroker::SetSecurityLevelForPluginProcess(int32_t aSandboxLevel) SANDBOX_ENSURE_SUCCESS(result, "With these static arguments AddRule should never fail, what happened?"); - // The following is required for the Java plugin. - result = mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES, - sandbox::TargetPolicy::FILES_ALLOW_ANY, - L"\\??\\pipe\\jpi2_pid*_pipe*"); - SANDBOX_ENSURE_SUCCESS(result, - "With these static arguments AddRule should never fail, what happened?"); - // These register keys are used by the file-browser dialog box. They // remember the most-recently-used folders. result = mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_REGISTRY, @@ -798,45 +830,6 @@ SandboxBroker::AllowReadFile(wchar_t const *file) return true; } -bool -SandboxBroker::AllowReadWriteFile(wchar_t const *file) -{ - if (!mPolicy) { - return false; - } - - auto result = - mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES, - sandbox::TargetPolicy::FILES_ALLOW_ANY, - file); - if (sandbox::SBOX_ALL_OK != result) { - LOG_E("Failed (ResultCode %d) to add read/write access to: %S", - result, file); - return false; - } - - return true; -} - -bool -SandboxBroker::AllowDirectory(wchar_t const *dir) -{ - if (!mPolicy) { - return false; - } - - auto result = - mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES, - sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, - dir); - if (sandbox::SBOX_ALL_OK != result) { - LOG_E("Failed (ResultCode %d) to add directory access to: %S", result, dir); - return false; - } - - return true; -} - bool SandboxBroker::AddTargetPeer(HANDLE aPeerProcess) { diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h index 9f5f62bc0a5c..598d0aab47bf 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h @@ -55,8 +55,6 @@ public: // File system permissions bool AllowReadFile(wchar_t const *file); - bool AllowReadWriteFile(wchar_t const *file); - bool AllowDirectory(wchar_t const *dir); // Exposes AddTargetPeer from broker services, so that none sandboxed // processes can be added as handle duplication targets.