From 8e6405075f72659abf6db415c64c0077f101de7d Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 2 May 2017 11:07:10 -0400 Subject: [PATCH] Bug 1361733 - In debug builds, do not allow content sandbox to write to all of /private/var r=haik This permission was needed for the memory bloat logging, which is used for leaktest, including logging intentionally crashing processes. Now we restrict ourselves to only allowing writes to the location needed for this logging, rather than all of /private/var. MozReview-Commit-ID: 5AbJEZlDHNV --- dom/ipc/ContentChild.cpp | 37 ++++++++++++++++++++++++++ security/sandbox/mac/Sandbox.h | 4 ++- security/sandbox/mac/Sandbox.mm | 10 +++---- security/sandbox/mac/SandboxPolicies.h | 13 +++++---- 4 files changed, 51 insertions(+), 13 deletions(-) diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 3141fadfcd91a..7dced323eab0e 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -1279,6 +1279,30 @@ IsDevelopmentBuild() return path == nullptr; } +// This function is only used in an |#ifdef DEBUG| path. +#ifdef DEBUG +// Given a path to a file, return the directory which contains it. +static nsAutoCString +GetDirectoryPath(const char *aPath) { + nsCOMPtr file = do_CreateInstance(NS_LOCAL_FILE_CONTRACTID); + if (!file || + NS_FAILED(file->InitWithNativePath(nsDependentCString(aPath)))) { + MOZ_CRASH("Failed to create or init an nsIFile"); + } + nsCOMPtr directoryFile; + if (NS_FAILED(file->GetParent(getter_AddRefs(directoryFile))) || + !directoryFile) { + MOZ_CRASH("Failed to get parent for an nsIFile"); + } + directoryFile->Normalize(); + nsAutoCString directoryPath; + if (NS_FAILED(directoryFile->GetNativePath(directoryPath))) { + MOZ_CRASH("Failed to get path for an nsIFile"); + } + return directoryPath; +} +#endif // DEBUG + static bool StartMacOSContentSandbox() { @@ -1352,6 +1376,19 @@ StartMacOSContentSandbox() info.hasSandboxedProfile = false; } +#ifdef DEBUG + // When a content process dies intentionally (|NoteIntentionalCrash|), for + // tests it wants to log that it did this. Allow writing to this location + // that the testrunner wants. + char *bloatLog = PR_GetEnv("XPCOM_MEM_BLOAT_LOG"); + if (bloatLog != nullptr) { + // |bloatLog| points to a specific file, but we actually write to a sibling + // of that path. + nsAutoCString bloatDirectoryPath = GetDirectoryPath(bloatLog); + info.debugWriteDir.assign(bloatDirectoryPath.get()); + } +#endif // DEBUG + std::string err; if (!mozilla::StartMacSandbox(info, err)) { NS_WARNING(err.c_str()); diff --git a/security/sandbox/mac/Sandbox.h b/security/sandbox/mac/Sandbox.h index bd158df8b7dff..0066250c7e527 100644 --- a/security/sandbox/mac/Sandbox.h +++ b/security/sandbox/mac/Sandbox.h @@ -48,7 +48,8 @@ typedef struct _MacSandboxInfo { pluginInfo(other.pluginInfo), appPath(other.appPath), appBinaryPath(other.appBinaryPath), appDir(other.appDir), appTempDir(other.appTempDir), - profileDir(other.profileDir), shouldLog(other.shouldLog) {} + profileDir(other.profileDir), debugWriteDir(other.debugWriteDir), + shouldLog(other.shouldLog) {} MacSandboxType type; int32_t level; bool hasFilePrivileges; @@ -59,6 +60,7 @@ typedef struct _MacSandboxInfo { std::string appDir; std::string appTempDir; std::string profileDir; + std::string debugWriteDir; bool shouldLog; } MacSandboxInfo; diff --git a/security/sandbox/mac/Sandbox.mm b/security/sandbox/mac/Sandbox.mm index 3aba150e3fa50..c23b4db309bea 100644 --- a/security/sandbox/mac/Sandbox.mm +++ b/security/sandbox/mac/Sandbox.mm @@ -178,12 +178,12 @@ bool StartMacSandbox(MacSandboxInfo aInfo, std::string &aErrorMessage) params.push_back(aInfo.hasSandboxedProfile ? "TRUE" : "FALSE"); params.push_back("HAS_FILE_PRIVILEGES"); params.push_back(aInfo.hasFilePrivileges ? "TRUE" : "FALSE"); - params.push_back("DEBUG_BUILD"); #ifdef DEBUG - params.push_back("TRUE"); -#else - params.push_back("FALSE"); -#endif + if (!aInfo.debugWriteDir.empty()) { + params.push_back("DEBUG_WRITE_DIR"); + params.push_back(aInfo.debugWriteDir.c_str()); + } +#endif // DEBUG } else { fprintf(stderr, "Content sandbox disabled due to sandbox level setting\n"); diff --git a/security/sandbox/mac/SandboxPolicies.h b/security/sandbox/mac/SandboxPolicies.h index 2af061534795b..0f6608940976c 100644 --- a/security/sandbox/mac/SandboxPolicies.h +++ b/security/sandbox/mac/SandboxPolicies.h @@ -49,7 +49,7 @@ static const char widevinePluginSandboxRulesAddend[] = R"( static const char contentSandboxRules[] = R"( (version 1) - (define should-log (param "SHOULD_LOG")) + (define should-log (param "SHOULD_LOG")) (define sandbox-level-1 (param "SANDBOX_LEVEL_1")) (define sandbox-level-2 (param "SANDBOX_LEVEL_2")) (define sandbox-level-3 (param "SANDBOX_LEVEL_3")) @@ -62,7 +62,7 @@ static const char contentSandboxRules[] = R"( (define profileDir (param "PROFILE_DIR")) (define home-path (param "HOME_PATH")) (define hasFilePrivileges (param "HAS_FILE_PRIVILEGES")) - (define isDebugBuild (param "DEBUG_BUILD")) + (define debugWriteDir (param "DEBUG_WRITE_DIR")) ; Allow read access to standard system paths. (allow file-read* @@ -224,8 +224,8 @@ static const char contentSandboxRules[] = R"( (subpath "/private/var/folders")) ; bug 1303987 - (if (string=? isDebugBuild "TRUE") - (allow file-write* (subpath "/private/var"))) + (if (string? debugWriteDir) + (allow file-write* (subpath debugWriteDir))) ; bug 1324610 (allow network-outbound (literal "/private/var/run/cupsd")) @@ -322,10 +322,9 @@ static const char contentSandboxRules[] = R"( (iokit-user-client-class "Gen6DVDContext")) ; bug 1237847 - (allow file-read* - (subpath appTempDir)) - (allow file-write* + (allow file-read* file-write* (subpath appTempDir)) + ) )";