# HG changeset patch # User Gian-Carlo Pascutto # Date 1515402436 -3600 # Mon Jan 08 10:07:16 2018 +0100 # Node ID 205e7ae2a6bc5ed1cdd1a982a12d99f52ce33258 # Parent a89071894b4904a0130139a03147d4a6cb5c3bfc Bug 1297740. diff --git a/sandbox/win/src/broker_services.cc b/sandbox/win/src/broker_services.cc --- a/sandbox/win/src/broker_services.cc +++ b/sandbox/win/src/broker_services.cc @@ -414,16 +414,17 @@ DWORD WINAPI BrokerServicesBase::TargetE } return policy; } // SpawnTarget does all the interesting sandbox setup and creates the target // process inside the sandbox. ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, const wchar_t* command_line, + base::EnvironmentMap& env_map, std::unique_ptr policy, DWORD* last_error, PROCESS_INFORMATION* target_info) { if (!exe_path) return SBOX_ERROR_BAD_PARAMS; // This code should only be called from the exe, ensure that this is always // the case. @@ -609,17 +610,17 @@ ResultCode BrokerServicesBase::SpawnTarg // Create the TargetProcess object and spawn the target suspended. Note that // Brokerservices does not own the target object. It is owned by the Policy. base::win::ScopedProcessInformation process_info; std::unique_ptr target = std::make_unique( std::move(*initial_token), std::move(*lockdown_token), thread_pool_); result = target->Create(exe_path, command_line, std::move(startup_info), - &process_info, last_error); + &process_info, env_map, last_error); if (result != SBOX_ALL_OK) { target->Terminate(); return result; } if (config_base->GetJobLevel() <= JobLevel::kLimitedUser) { // Restrict the job from containing any processes. Job restrictions diff --git a/sandbox/win/src/broker_services.h b/sandbox/win/src/broker_services.h --- a/sandbox/win/src/broker_services.h +++ b/sandbox/win/src/broker_services.h @@ -8,16 +8,17 @@ #include #include #include #include #include #include "base/compiler_specific.h" #include "base/containers/flat_map.h" +#include "base/environment.h" #include "base/memory/raw_ptr.h" #include "base/memory/scoped_refptr.h" #include "base/win/scoped_handle.h" #include "sandbox/win/src/alternate_desktop.h" #include "sandbox/win/src/crosscall_server.h" #include "sandbox/win/src/sandbox.h" #include "sandbox/win/src/sandbox_policy_base.h" #include "sandbox/win/src/sharedmem_ipc_server.h" @@ -39,16 +40,17 @@ class BrokerServicesBase final : public std::unique_ptr target_tracker) override; ResultCode CreateAlternateDesktop(Desktop desktop) override; void DestroyDesktops() override; std::unique_ptr CreatePolicy() override; std::unique_ptr CreatePolicy(base::StringPiece key) override; ResultCode SpawnTarget(const wchar_t* exe_path, const wchar_t* command_line, + base::EnvironmentMap& env_map, std::unique_ptr policy, DWORD* last_error, PROCESS_INFORMATION* target) override; ResultCode GetPolicyDiagnostics( std::unique_ptr receiver) override; void SetStartingMitigations(MitigationFlags starting_mitigations) override; bool RatchetDownSecurityMitigations( MitigationFlags additional_flags) override; diff --git a/sandbox/win/src/sandbox.h b/sandbox/win/src/sandbox.h --- a/sandbox/win/src/sandbox.h +++ b/sandbox/win/src/sandbox.h @@ -84,16 +84,17 @@ class BrokerServices { // parameter will hold the last Win32 error value. // target: returns the resulting target process information such as process // handle and PID just as if CreateProcess() had been called. The caller is // responsible for closing the handles returned in this structure. // Returns: // ALL_OK if successful. All other return values imply failure. virtual ResultCode SpawnTarget(const wchar_t* exe_path, const wchar_t* command_line, + base::EnvironmentMap& env_map, std::unique_ptr policy, DWORD* last_error, PROCESS_INFORMATION* target) = 0; // This call creates a snapshot of policies managed by the sandbox and // returns them via a helper class. // Parameters: // receiver: The |PolicyDiagnosticsReceiver| implementation will be diff --git a/sandbox/win/src/target_process.cc b/sandbox/win/src/target_process.cc --- a/sandbox/win/src/target_process.cc +++ b/sandbox/win/src/target_process.cc @@ -9,16 +9,17 @@ #include #include #include #include #include "base/containers/span.h" #include "base/memory/free_deleter.h" #include "base/numerics/safe_conversions.h" +#include "base/process/environment_internal.h" #include "base/strings/string_util.h" #include "base/win/access_token.h" #include "base/win/current_module.h" #include "base/win/scoped_handle.h" #include "base/win/security_util.h" #include "base/win/startup_information.h" #include "sandbox/win/src/crosscall_client.h" #include "sandbox/win/src/crosscall_server.h" @@ -137,16 +138,17 @@ TargetProcess::~TargetProcess() { // Creates the target (child) process suspended and assigns it to the job // object. ResultCode TargetProcess::Create( const wchar_t* exe_path, const wchar_t* command_line, std::unique_ptr startup_info_helper, base::win::ScopedProcessInformation* target_info, + base::EnvironmentMap& env_changes, DWORD* win_error) { exe_name_.reset(_wcsdup(exe_path)); base::win::StartupInformation* startup_info = startup_info_helper->GetStartupInformation(); // the command line needs to be writable by CreateProcess(). std::unique_ptr cmd_line(_wcsdup(command_line)); @@ -148,40 +150,48 @@ ResultCode TargetProcess::Create( DWORD flags = CREATE_SUSPENDED | CREATE_UNICODE_ENVIRONMENT | DETACHED_PROCESS; if (startup_info->has_extended_startup_info()) flags |= EXTENDED_STARTUPINFO_PRESENT; std::wstring new_env; + wchar_t* old_environment = ::GetEnvironmentStringsW(); + if (!old_environment) { + return SBOX_ERROR_CANNOT_OBTAIN_ENVIRONMENT; + } + if (startup_info_helper->IsEnvironmentFiltered()) { - wchar_t* old_environment = ::GetEnvironmentStringsW(); - if (!old_environment) { - return SBOX_ERROR_CANNOT_OBTAIN_ENVIRONMENT; - } - // Only copy a limited list of variables to the target from the broker's // environment. These are // * "Path", "SystemDrive", "SystemRoot", "TEMP", "TMP": Needed for normal // operation and tests. // * "LOCALAPPDATA": Needed for App Container processes. // * "CHROME_CRASHPAD_PIPE_NAME": Needed for crashpad. static constexpr base::WStringPiece to_keep[] = { L"Path", L"SystemDrive", L"SystemRoot", L"TEMP", L"TMP", L"LOCALAPPDATA", L"CHROME_CRASHPAD_PIPE_NAME"}; new_env = FilterEnvironment(old_environment, to_keep); - ::FreeEnvironmentStringsW(old_environment); + } else { + // Environment strings block is terminated by a double null. + wchar_t* end_ptr = old_environment; + while (*end_ptr++ || *end_ptr++) { + } + new_env.assign(old_environment, end_ptr); } + + ::FreeEnvironmentStringsW(old_environment); + new_env = base::internal::AlterEnvironment(std::data(new_env), env_changes); bool inherit_handles = startup_info_helper->ShouldInheritHandles(); PROCESS_INFORMATION temp_process_info = {}; if (!::CreateProcessAsUserW(lockdown_token_.get(), exe_path, cmd_line.get(), nullptr, // No security attribute. nullptr, // No thread attribute. inherit_handles, flags, new_env.empty() ? nullptr : std::data(new_env), diff --git a/sandbox/win/src/target_process.h b/sandbox/win/src/target_process.h --- a/sandbox/win/src/target_process.h +++ b/sandbox/win/src/target_process.h @@ -6,16 +6,17 @@ #define SANDBOX_WIN_SRC_TARGET_PROCESS_H_ #include #include #include #include "base/containers/span.h" +#include "base/environment.h" #include "base/gtest_prod_util.h" #include "base/memory/free_deleter.h" #include "base/memory/raw_ptr.h" #include "base/strings/string_util.h" #include "base/win/access_token.h" #include "base/win/scoped_handle.h" #include "base/win/scoped_process_information.h" #include "base/win/sid.h" @@ -54,16 +55,17 @@ class TargetProcess { ~TargetProcess(); // Creates the new target process. The process is created suspended. ResultCode Create(const wchar_t* exe_path, const wchar_t* command_line, std::unique_ptr startup_info, base::win::ScopedProcessInformation* target_info, + base::EnvironmentMap& env_map, DWORD* win_error); // Destroys the target process. void Terminate(); // Creates the IPC objects such as the BrokerDispatcher and the // IPC server. The IPC server uses the services of the thread_pool. ResultCode Init(Dispatcher* ipc_dispatcher,