diff options
author | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-04 20:16:32 +0000 |
---|---|---|
committer | erikwright@chromium.org <erikwright@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-04-04 20:16:32 +0000 |
commit | 362b88198c4713730eaa93da708a33ab56a0484e (patch) | |
tree | 6ce96a4a6fe4a367cdd5c337bf390a4bd1edb79a /sandbox | |
parent | 519fbd840c13c53aaa01fd30cbbeebb134d09a42 (diff) | |
download | chromium_src-362b88198c4713730eaa93da708a33ab56a0484e.zip chromium_src-362b88198c4713730eaa93da708a33ab56a0484e.tar.gz chromium_src-362b88198c4713730eaa93da708a33ab56a0484e.tar.bz2 |
Use ScopedProcessInformation and other RAII types in sandbox.
See http://codereview.chromium.org/9700038/ for the definition of ScopedProcessInformation.
BUG=None
TEST=None
Review URL: https://chromiumcodereview.appspot.com/9959018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@130716 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sandbox')
-rw-r--r-- | sandbox/src/Wow64.cc | 15 | ||||
-rw-r--r-- | sandbox/src/broker_services.cc | 45 | ||||
-rw-r--r-- | sandbox/src/interception_unittest.cc | 18 | ||||
-rw-r--r-- | sandbox/src/job_unittest.cc | 13 | ||||
-rw-r--r-- | sandbox/src/policy_target_test.cc | 43 | ||||
-rw-r--r-- | sandbox/src/process_policy_test.cc | 23 | ||||
-rw-r--r-- | sandbox/src/restricted_token_utils.cc | 27 | ||||
-rw-r--r-- | sandbox/src/target_process.cc | 110 | ||||
-rw-r--r-- | sandbox/src/target_process.h | 37 |
9 files changed, 163 insertions, 168 deletions
diff --git a/sandbox/src/Wow64.cc b/sandbox/src/Wow64.cc index 60c63fd..9f10a4b 100644 --- a/sandbox/src/Wow64.cc +++ b/sandbox/src/Wow64.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,6 +8,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "base/win/scoped_process_information.h" #include "base/win/windows_version.h" #include "sandbox/src/target_process.h" @@ -156,18 +157,16 @@ bool Wow64::RunWowHelper(void* buffer) { STARTUPINFO startup_info = {0}; startup_info.cb = sizeof(startup_info); - PROCESS_INFORMATION process_info; + base::win::ScopedProcessInformation process_info; if (!::CreateProcess(NULL, writable_command.get(), NULL, NULL, FALSE, 0, NULL, - NULL, &startup_info, &process_info)) + NULL, &startup_info, process_info.Receive())) return false; - DWORD reason = ::WaitForSingleObject(process_info.hProcess, INFINITE); + DWORD reason = ::WaitForSingleObject(process_info.process_handle(), INFINITE); DWORD code; - bool ok = ::GetExitCodeProcess(process_info.hProcess, &code) ? true : false; - - ::CloseHandle(process_info.hProcess); - ::CloseHandle(process_info.hThread); + bool ok = + ::GetExitCodeProcess(process_info.process_handle(), &code) ? true : false; if (WAIT_TIMEOUT == reason) return false; diff --git a/sandbox/src/broker_services.cc b/sandbox/src/broker_services.cc index ff5be3a..d361c2e 100644 --- a/sandbox/src/broker_services.cc +++ b/sandbox/src/broker_services.cc @@ -5,7 +5,10 @@ #include "sandbox/src/broker_services.h" #include "base/logging.h" +#include "base/memory/scoped_ptr.h" #include "base/threading/platform_thread.h" +#include "base/win/scoped_handle.h" +#include "base/win/scoped_process_information.h" #include "sandbox/src/sandbox_policy_base.h" #include "sandbox/src/sandbox.h" #include "sandbox/src/target_process.h" @@ -245,14 +248,15 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, // Construct the tokens and the job object that we are going to associate // with the soon to be created target process. - HANDLE lockdown_token = NULL; - HANDLE initial_token = NULL; - DWORD win_result = policy_base->MakeTokens(&initial_token, &lockdown_token); + base::win::ScopedHandle lockdown_token; + base::win::ScopedHandle initial_token; + DWORD win_result = policy_base->MakeTokens(initial_token.Receive(), + lockdown_token.Receive()); if (ERROR_SUCCESS != win_result) return SBOX_ERROR_GENERIC; - HANDLE job = NULL; - win_result = policy_base->MakeJobObject(&job); + base::win::ScopedHandle job; + win_result = policy_base->MakeJobObject(job.Receive()); if (ERROR_SUCCESS != win_result) return SBOX_ERROR_GENERIC; @@ -266,9 +270,11 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, // Create the TargetProces object and spawn the target suspended. Note that // Brokerservices does not own the target object. It is owned by the Policy. - PROCESS_INFORMATION process_info = {0}; - TargetProcess* target = new TargetProcess(initial_token, lockdown_token, - job, thread_pool_); + base::win::ScopedProcessInformation process_info; + TargetProcess* target = new TargetProcess(initial_token.Take(), + lockdown_token.Take(), + job, + thread_pool_); std::wstring desktop = policy_base->GetAlternateDesktop(); @@ -278,10 +284,6 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, if (ERROR_SUCCESS != win_result) return SpawnCleanup(target, win_result); - if ((INVALID_HANDLE_VALUE == process_info.hProcess) || - (INVALID_HANDLE_VALUE == process_info.hThread)) - return SpawnCleanup(target, win_result); - // Now the policy is the owner of the target. if (!policy_base->AddTarget(target)) { return SpawnCleanup(target, 0); @@ -290,24 +292,15 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, // We are going to keep a pointer to the policy because we'll call it when // the job object generates notifications using the completion port. policy_base->AddRef(); - JobTracker* tracker = new JobTracker(job, policy_base); - if (!AssociateCompletionPort(job, job_port_, tracker)) + scoped_ptr<JobTracker> tracker(new JobTracker(job.Take(), policy_base)); + if (!AssociateCompletionPort(tracker->job, job_port_, tracker.get())) return SpawnCleanup(target, 0); // Save the tracker because in cleanup we might need to force closing // the Jobs. - tracker_list_.push_back(tracker); - child_process_ids_.insert(process_info.dwProcessId); - - // We return the caller a duplicate of the process handle so they - // can close it at will. - HANDLE dup_process_handle = NULL; - if (!::DuplicateHandle(::GetCurrentProcess(), process_info.hProcess, - ::GetCurrentProcess(), &dup_process_handle, - 0, FALSE, DUPLICATE_SAME_ACCESS)) - return SpawnCleanup(target, 0); + tracker_list_.push_back(tracker.release()); + child_process_ids_.insert(process_info.process_id()); - *target_info = process_info; - target_info->hProcess = dup_process_handle; + *target_info = process_info.Take(); return SBOX_ALL_OK; } diff --git a/sandbox/src/interception_unittest.cc b/sandbox/src/interception_unittest.cc index a0dd98d..0dabb84 100644 --- a/sandbox/src/interception_unittest.cc +++ b/sandbox/src/interception_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -79,7 +79,13 @@ TEST(InterceptionManagerTest, BufferLayout1) { wchar_t exe_name[MAX_PATH]; ASSERT_NE(0u, GetModuleFileName(NULL, exe_name, MAX_PATH - 1)); - TargetProcess *target = MakeTestTargetProcess(::GetCurrentProcess(), + base::win::ScopedHandle current_process; + ASSERT_TRUE( + ::DuplicateHandle(::GetCurrentProcess(), ::GetCurrentProcess(), + ::GetCurrentProcess(), current_process.Receive(), + 0, FALSE, DUPLICATE_SAME_ACCESS)); + + TargetProcess *target = MakeTestTargetProcess(current_process.Take(), ::GetModuleHandle(exe_name)); InterceptionManager interceptions(target, true); @@ -166,7 +172,13 @@ TEST(InterceptionManagerTest, BufferLayout2) { wchar_t exe_name[MAX_PATH]; ASSERT_NE(0u, GetModuleFileName(NULL, exe_name, MAX_PATH - 1)); - TargetProcess *target = MakeTestTargetProcess(::GetCurrentProcess(), + base::win::ScopedHandle current_process; + ASSERT_TRUE( + ::DuplicateHandle(::GetCurrentProcess(), ::GetCurrentProcess(), + ::GetCurrentProcess(), current_process.Receive(), + 0, FALSE, DUPLICATE_SAME_ACCESS)); + + TargetProcess *target = MakeTestTargetProcess(current_process.Take(), ::GetModuleHandle(exe_name)); InterceptionManager interceptions(target, true); diff --git a/sandbox/src/job_unittest.cc b/sandbox/src/job_unittest.cc index 0f7010d..f386f1f 100644 --- a/sandbox/src/job_unittest.cc +++ b/sandbox/src/job_unittest.cc @@ -1,9 +1,10 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. // This file contains unit tests for the job object. +#include "base/win/scoped_process_information.h" #include "sandbox/src/job.h" #include "testing/gtest/include/gtest/gtest.h" @@ -159,11 +160,11 @@ TEST(JobTest, ProcessInJob) { wchar_t notepad[] = L"notepad"; STARTUPINFO si = { sizeof(si) }; - PROCESS_INFORMATION pi = {0}; + base::win::ScopedProcessInformation pi; result = ::CreateProcess(NULL, notepad, NULL, NULL, FALSE, 0, NULL, NULL, &si, - &pi); + pi.Receive()); ASSERT_TRUE(result); - ASSERT_EQ(ERROR_SUCCESS, job.AssignProcessToJob(pi.hProcess)); + ASSERT_EQ(ERROR_SUCCESS, job.AssignProcessToJob(pi.process_handle())); // Get the job handle. HANDLE job_handle = job.Detach(); @@ -178,9 +179,9 @@ TEST(JobTest, ProcessInJob) { EXPECT_EQ(1, jbpidl.NumberOfAssignedProcesses); EXPECT_EQ(1, jbpidl.NumberOfProcessIdsInList); - EXPECT_EQ(pi.dwProcessId, jbpidl.ProcessIdList[0]); + EXPECT_EQ(pi.process_id(), jbpidl.ProcessIdList[0]); - EXPECT_TRUE(::TerminateProcess(pi.hProcess, 0)); + EXPECT_TRUE(::TerminateProcess(pi.process_handle(), 0)); EXPECT_TRUE(::CloseHandle(job_handle)); } diff --git a/sandbox/src/policy_target_test.cc b/sandbox/src/policy_target_test.cc index 577c8c5..dd59ac8 100644 --- a/sandbox/src/policy_target_test.cc +++ b/sandbox/src/policy_target_test.cc @@ -1,7 +1,8 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/win/scoped_process_information.h" #include "base/win/windows_version.h" #include "sandbox/src/sandbox.h" #include "sandbox/src/sandbox_factory.h" @@ -149,9 +150,9 @@ SBOX_TESTS_COMMAND int PolicyTargetTest_process(int argc, wchar_t **argv) { // Use default values to create a new process. STARTUPINFO startup_info = {0}; startup_info.cb = sizeof(startup_info); - PROCESS_INFORMATION process_info; + base::win::ScopedProcessInformation process_info; if (!::CreateProcessW(L"foo.exe", L"foo.exe", NULL, NULL, FALSE, 0, - NULL, NULL, &startup_info, &process_info)) + NULL, NULL, &startup_info, process_info.Receive())) return SBOX_TEST_SUCCEEDED; return SBOX_TEST_FAILED; } @@ -233,33 +234,31 @@ TEST(PolicyTargetTest, DesktopPolicy) { // Launch the app. ResultCode result = SBOX_ALL_OK; - PROCESS_INFORMATION target = {0}; + base::win::ScopedProcessInformation target; TargetPolicy* policy = broker->CreatePolicy(); policy->SetAlternateDesktop(false); policy->SetTokenLevel(USER_INTERACTIVE, USER_LOCKDOWN); - result = broker->SpawnTarget(prog_name, arguments.c_str(), policy, &target); + result = broker->SpawnTarget( + prog_name, arguments.c_str(), policy, target.Receive()); policy->Release(); EXPECT_EQ(SBOX_ALL_OK, result); - EXPECT_EQ(1, ::ResumeThread(target.hThread)); + EXPECT_EQ(1, ::ResumeThread(target.thread_handle())); - EXPECT_EQ(WAIT_TIMEOUT, ::WaitForSingleObject(target.hProcess, 2000)); + EXPECT_EQ(WAIT_TIMEOUT, ::WaitForSingleObject(target.process_handle(), 2000)); - EXPECT_NE(::GetThreadDesktop(target.dwThreadId), + EXPECT_NE(::GetThreadDesktop(target.thread_id()), ::GetThreadDesktop(::GetCurrentThreadId())); std::wstring desktop_name = policy->GetAlternateDesktop(); HDESK desk = ::OpenDesktop(desktop_name.c_str(), 0, FALSE, DESKTOP_ENUMERATE); EXPECT_TRUE(NULL != desk); EXPECT_TRUE(::CloseDesktop(desk)); - EXPECT_TRUE(::TerminateProcess(target.hProcess, 0)); + EXPECT_TRUE(::TerminateProcess(target.process_handle(), 0)); - ::WaitForSingleObject(target.hProcess, INFINITE); - - EXPECT_TRUE(::CloseHandle(target.hProcess)); - EXPECT_TRUE(::CloseHandle(target.hThread)); + ::WaitForSingleObject(target.process_handle(), INFINITE); // Close the desktop handle. temp_policy = broker->CreatePolicy(); @@ -295,21 +294,22 @@ TEST(PolicyTargetTest, WinstaPolicy) { // Launch the app. ResultCode result = SBOX_ALL_OK; - PROCESS_INFORMATION target = {0}; + base::win::ScopedProcessInformation target; TargetPolicy* policy = broker->CreatePolicy(); policy->SetAlternateDesktop(true); policy->SetTokenLevel(USER_INTERACTIVE, USER_LOCKDOWN); - result = broker->SpawnTarget(prog_name, arguments.c_str(), policy, &target); + result = broker->SpawnTarget( + prog_name, arguments.c_str(), policy, target.Receive()); policy->Release(); EXPECT_EQ(SBOX_ALL_OK, result); - EXPECT_EQ(1, ::ResumeThread(target.hThread)); + EXPECT_EQ(1, ::ResumeThread(target.thread_handle())); - EXPECT_EQ(WAIT_TIMEOUT, ::WaitForSingleObject(target.hProcess, 2000)); + EXPECT_EQ(WAIT_TIMEOUT, ::WaitForSingleObject(target.process_handle(), 2000)); - EXPECT_NE(::GetThreadDesktop(target.dwThreadId), + EXPECT_NE(::GetThreadDesktop(target.thread_id()), ::GetThreadDesktop(::GetCurrentThreadId())); std::wstring desktop_name = policy->GetAlternateDesktop(); @@ -324,12 +324,9 @@ TEST(PolicyTargetTest, WinstaPolicy) { HDESK desk = ::OpenDesktop(desktop_name.c_str(), 0, FALSE, DESKTOP_ENUMERATE); // This should fail if the desktop is really on another window station. EXPECT_FALSE(NULL != desk); - EXPECT_TRUE(::TerminateProcess(target.hProcess, 0)); - - ::WaitForSingleObject(target.hProcess, INFINITE); + EXPECT_TRUE(::TerminateProcess(target.process_handle(), 0)); - EXPECT_TRUE(::CloseHandle(target.hProcess)); - EXPECT_TRUE(::CloseHandle(target.hThread)); + ::WaitForSingleObject(target.process_handle(), INFINITE); // Close the desktop handle. temp_policy = broker->CreatePolicy(); diff --git a/sandbox/src/process_policy_test.cc b/sandbox/src/process_policy_test.cc index 1bcfa88..783446e 100644 --- a/sandbox/src/process_policy_test.cc +++ b/sandbox/src/process_policy_test.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -7,6 +7,7 @@ #include "base/sys_string_conversions.h" #include "base/win/scoped_handle.h" +#include "base/win/scoped_process_information.h" #include "sandbox/src/sandbox.h" #include "sandbox/src/sandbox_policy.h" #include "sandbox/src/sandbox_factory.h" @@ -34,7 +35,7 @@ std::wstring MakeFullPathToSystem32(const wchar_t* name) { // unicode and ascii version of the api. sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe, const std::wstring &command) { - PROCESS_INFORMATION pi; + base::win::ScopedProcessInformation pi; STARTUPINFOW si = {sizeof(si)}; const wchar_t *exe_name = NULL; @@ -48,7 +49,7 @@ sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe, // Create the process with the unicode version of the API. sandbox::SboxTestResult ret1 = sandbox::SBOX_TEST_FAILED; if (!::CreateProcessW(exe_name, const_cast<wchar_t*>(cmd_line), NULL, NULL, - FALSE, 0, NULL, NULL, &si, &pi)) { + FALSE, 0, NULL, NULL, &si, pi.Receive())) { DWORD last_error = GetLastError(); if ((ERROR_NOT_ENOUGH_QUOTA == last_error) || (ERROR_ACCESS_DENIED == last_error) || @@ -61,6 +62,8 @@ sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe, ret1 = sandbox::SBOX_TEST_SUCCEEDED; } + pi.Close(); + // Do the same with the ansi version of the api STARTUPINFOA sia = {sizeof(sia)}; sandbox::SboxTestResult ret2 = sandbox::SBOX_TEST_FAILED; @@ -71,7 +74,7 @@ sandbox::SboxTestResult CreateProcessHelper(const std::wstring &exe, if (!::CreateProcessA( exe_name ? base::SysWideToMultiByte(exe_name, CP_UTF8).c_str() : NULL, cmd_line ? const_cast<char*>(narrow_cmd_line.c_str()) : NULL, - NULL, NULL, FALSE, 0, NULL, NULL, &sia, &pi)) { + NULL, NULL, FALSE, 0, NULL, NULL, &sia, pi.Receive())) { DWORD last_error = GetLastError(); if ((ERROR_NOT_ENOUGH_QUOTA == last_error) || (ERROR_ACCESS_DENIED == last_error) || @@ -170,24 +173,22 @@ SBOX_TESTS_COMMAND int Process_GetChildProcessToken(int argc, wchar_t **argv) { std::wstring path = MakeFullPathToSystem32(argv[0]); - PROCESS_INFORMATION pi = {0}; + base::win::ScopedProcessInformation pi; STARTUPINFOW si = {sizeof(si)}; if (!::CreateProcessW(path.c_str(), NULL, NULL, NULL, FALSE, CREATE_SUSPENDED, - NULL, NULL, &si, &pi)) { + NULL, NULL, &si, pi.Receive())) { return SBOX_TEST_FAILED; } - base::win::ScopedHandle process(pi.hProcess); - base::win::ScopedHandle thread(pi.hThread); - HANDLE token = NULL; - BOOL result = ::OpenProcessToken(process.Get(), TOKEN_IMPERSONATE, &token); + BOOL result = + ::OpenProcessToken(pi.process_handle(), TOKEN_IMPERSONATE, &token); DWORD error = ::GetLastError(); base::win::ScopedHandle token_handle(token); - if (!::TerminateProcess(process.Get(), 0)) + if (!::TerminateProcess(pi.process_handle(), 0)) return SBOX_TEST_FAILED; if (result && token) diff --git a/sandbox/src/restricted_token_utils.cc b/sandbox/src/restricted_token_utils.cc index b036e51..d327c5f 100644 --- a/sandbox/src/restricted_token_utils.cc +++ b/sandbox/src/restricted_token_utils.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -10,6 +10,7 @@ #include "base/logging.h" #include "base/win/scoped_handle.h" +#include "base/win/scoped_process_information.h" #include "base/win/windows_version.h" #include "sandbox/src/job.h" #include "sandbox/src/restricted_token.h" @@ -182,7 +183,7 @@ DWORD StartRestrictedProcessInJob(wchar_t *command_line, // Start the process STARTUPINFO startup_info = {0}; - PROCESS_INFORMATION process_info = {0}; + base::win::ScopedProcessInformation process_info; if (!::CreateProcessAsUser(primary_token.Get(), NULL, // No application name. @@ -194,30 +195,30 @@ DWORD StartRestrictedProcessInJob(wchar_t *command_line, NULL, // Use the environment of the caller. NULL, // Use current directory of the caller. &startup_info, - &process_info)) { + process_info.Receive())) { return ::GetLastError(); } - base::win::ScopedHandle thread_handle(process_info.hThread); - base::win::ScopedHandle process_handle(process_info.hProcess); - // Change the token of the main thread of the new process for the // impersonation token with more rights. - if (!::SetThreadToken(&process_info.hThread, impersonation_token.Get())) { - ::TerminateProcess(process_handle.Get(), - 0); // exit code - return ::GetLastError(); + { + HANDLE temp_thread = process_info.thread_handle(); + if (!::SetThreadToken(&temp_thread, impersonation_token.Get())) { + ::TerminateProcess(process_info.process_handle(), + 0); // exit code + return ::GetLastError(); + } } - err_code = job.AssignProcessToJob(process_handle.Get()); + err_code = job.AssignProcessToJob(process_info.process_handle()); if (ERROR_SUCCESS != err_code) { - ::TerminateProcess(process_handle.Get(), + ::TerminateProcess(process_info.process_handle(), 0); // exit code return ::GetLastError(); } // Start the application - ::ResumeThread(thread_handle.Get()); + ::ResumeThread(process_info.thread_handle()); (*job_handle_ret) = job.Detach(); diff --git a/sandbox/src/target_process.cc b/sandbox/src/target_process.cc index 2710dc0..1e3f9a0 100644 --- a/sandbox/src/target_process.cc +++ b/sandbox/src/target_process.cc @@ -15,12 +15,6 @@ namespace { -void TerminateTarget(PROCESS_INFORMATION* pi) { - ::CloseHandle(pi->hThread); - ::TerminateProcess(pi->hProcess, 0); - ::CloseHandle(pi->hProcess); -} - void CopyPolicyToTarget(const void* source, size_t size, void* dest) { if (!source || !size) return; @@ -97,14 +91,8 @@ TargetProcess::TargetProcess(HANDLE initial_token, HANDLE lockdown_token, : lockdown_token_(lockdown_token), initial_token_(initial_token), job_(job), - shared_section_(NULL), - ipc_server_(NULL), thread_pool_(thread_pool), - base_address_(NULL), - exe_name_(NULL), - sandbox_process_(NULL), - sandbox_thread_(NULL), - sandbox_process_id_(0) { + base_address_(NULL) { } TargetProcess::~TargetProcess() { @@ -117,23 +105,18 @@ TargetProcess::~TargetProcess() { // it. http://b/893891 // For now, this wait is there only to do a best effort to prevent some leaks // from showing up in purify. - ::WaitForSingleObject(sandbox_process_, 50); - if (!::GetExitCodeProcess(sandbox_process_, &exit_code) || - (STILL_ACTIVE == exit_code)) { + ::WaitForSingleObject(sandbox_process_info_.process_handle(), 50); + if (!::GetExitCodeProcess(sandbox_process_info_.process_handle(), + &exit_code) || (STILL_ACTIVE == exit_code)) { // It is an error to destroy this object while the target process is still // alive because we need to destroy the IPC subsystem and cannot risk to // have an IPC reach us after this point. return; } - delete ipc_server_; - - ::CloseHandle(lockdown_token_); - ::CloseHandle(initial_token_); - ::CloseHandle(sandbox_process_); - if (shared_section_) - ::CloseHandle(shared_section_); - free(exe_name_); + // ipc_server_ references our process handle, so make sure the former is shut + // down before the latter is closed (by ScopedProcessInformation). + ipc_server_.reset(); } // Creates the target (child) process suspended and assigns it to the job @@ -141,8 +124,8 @@ TargetProcess::~TargetProcess() { DWORD TargetProcess::Create(const wchar_t* exe_path, const wchar_t* command_line, const wchar_t* desktop, - PROCESS_INFORMATION* target_info) { - exe_name_ = _wcsdup(exe_path); + base::win::ScopedProcessInformation* target_info) { + exe_name_.reset(_wcsdup(exe_path)); // the command line needs to be writable by CreateProcess(). scoped_ptr_malloc<wchar_t> cmd_line(_wcsdup(command_line)); @@ -157,7 +140,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, startup_info.lpDesktop = desktop_name.get(); } - PROCESS_INFORMATION process_info = {0}; + base::win::ScopedProcessInformation process_info; if (!::CreateProcessAsUserW(lockdown_token_, exe_path, @@ -169,44 +152,43 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, NULL, // Use the environment of the caller. NULL, // Use current directory of the caller. &startup_info, - &process_info)) { + process_info.Receive())) { return ::GetLastError(); } - PoisonLowerAddressRange(process_info.hProcess); + PoisonLowerAddressRange(process_info.process_handle()); DWORD win_result = ERROR_SUCCESS; // Assign the suspended target to the windows job object - if (!::AssignProcessToJobObject(job_, process_info.hProcess)) { + if (!::AssignProcessToJobObject(job_, process_info.process_handle())) { win_result = ::GetLastError(); // It might be a security breach if we let the target run outside the job // so kill it before it causes damage - TerminateTarget(&process_info); + ::TerminateProcess(process_info.process_handle(), 0); return win_result; } // Change the token of the main thread of the new process for the // impersonation token with more rights. This allows the target to start; // otherwise it will crash too early for us to help. - if (!SetThreadToken(&process_info.hThread, initial_token_)) { - win_result = ::GetLastError(); - TerminateTarget(&process_info); - return win_result; + { + HANDLE temp_thread = process_info.thread_handle(); + if (!::SetThreadToken(&temp_thread, initial_token_)) { + win_result = ::GetLastError(); + ::TerminateProcess(process_info.process_handle(), 0); + return win_result; + } } CONTEXT context; context.ContextFlags = CONTEXT_ALL; - if (!::GetThreadContext(process_info.hThread, &context)) { + if (!::GetThreadContext(process_info.thread_handle(), &context)) { win_result = ::GetLastError(); - TerminateTarget(&process_info); + ::TerminateProcess(process_info.process_handle(), 0); return win_result; } - sandbox_process_ = process_info.hProcess; - sandbox_thread_ = process_info.hThread; - sandbox_process_id_ = process_info.dwProcessId; - #if defined(_WIN64) void* entry_point = reinterpret_cast<void*>(context.Rcx); #else @@ -216,20 +198,26 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, void* entry_point = reinterpret_cast<void*>(context.Eax); #pragma warning(pop) #endif // _WIN64 + + if (!target_info->DuplicateFrom(process_info)) { + ::TerminateProcess(process_info.process_handle(), 0); + return ERROR_INVALID_FUNCTION; + } + base_address_ = GetBaseAddress(exe_path, entry_point); - *target_info = process_info; + sandbox_process_info_.Swap(&process_info); return win_result; } ResultCode TargetProcess::TransferVariable(char* name, void* address, size_t size) { - if (NULL == sandbox_process_) + if (!sandbox_process_info_.IsValid()) return SBOX_ERROR_UNEXPECTED_CALL; void* child_var = address; #if SANDBOX_EXPORTS - HMODULE module = ::LoadLibrary(exe_name_); + HMODULE module = ::LoadLibrary(exe_name_.get()); if (NULL == module) return SBOX_ERROR_GENERIC; @@ -247,8 +235,8 @@ ResultCode TargetProcess::TransferVariable(char* name, void* address, #endif SIZE_T written; - if (!::WriteProcessMemory(sandbox_process_, child_var, address, size, - &written)) + if (!::WriteProcessMemory(sandbox_process_info_.process_handle(), + child_var, address, size, &written)) return SBOX_ERROR_GENERIC; if (written != size) @@ -270,18 +258,18 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy, // We use this single memory pool for IPC and for policy. DWORD shared_mem_size = static_cast<DWORD>(shared_IPC_size + shared_policy_size); - shared_section_ = ::CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, - PAGE_READWRITE | SEC_COMMIT, - 0, shared_mem_size, NULL); - if (NULL == shared_section_) { + shared_section_.Set(::CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, + PAGE_READWRITE | SEC_COMMIT, + 0, shared_mem_size, NULL)); + if (!shared_section_.IsValid()) { return ::GetLastError(); } DWORD access = FILE_MAP_READ | FILE_MAP_WRITE; - HANDLE target_shared_section = NULL; + base::win::ScopedHandle target_shared_section; if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_, - sandbox_process_, &target_shared_section, - access, FALSE, 0)) { + sandbox_process_info_.process_handle(), + target_shared_section.Receive(), access, FALSE, 0)) { return ::GetLastError(); } @@ -297,7 +285,7 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy, ResultCode ret; // Set the global variables in the target. These are not used on the broker. - g_shared_section = target_shared_section; + g_shared_section = target_shared_section.Take(); ret = TransferVariable("g_shared_section", &g_shared_section, sizeof(g_shared_section)); g_shared_section = NULL; @@ -322,29 +310,31 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy, ::GetLastError() : ERROR_INVALID_FUNCTION; } - ipc_server_ = new SharedMemIPCServer(sandbox_process_, sandbox_process_id_, - job_, thread_pool_, ipc_dispatcher); + ipc_server_.reset( + new SharedMemIPCServer(sandbox_process_info_.process_handle(), + sandbox_process_info_.process_id(), + job_, thread_pool_, ipc_dispatcher)); if (!ipc_server_->Init(shared_memory, shared_IPC_size, kIPCChannelSize)) return ERROR_NOT_ENOUGH_MEMORY; // After this point we cannot use this handle anymore. - sandbox_thread_ = NULL; + ::CloseHandle(sandbox_process_info_.TakeThreadHandle()); return ERROR_SUCCESS; } void TargetProcess::Terminate() { - if (NULL == sandbox_process_) + if (!sandbox_process_info_.IsValid()) return; - ::TerminateProcess(sandbox_process_, 0); + ::TerminateProcess(sandbox_process_info_.process_handle(), 0); } TargetProcess* MakeTestTargetProcess(HANDLE process, HMODULE base_address) { TargetProcess* target = new TargetProcess(NULL, NULL, NULL, NULL); - target->sandbox_process_ = process; + target->sandbox_process_info_.Receive()->hProcess = process; target->base_address_ = base_address; return target; } diff --git a/sandbox/src/target_process.h b/sandbox/src/target_process.h index f20935d..5afe654 100644 --- a/sandbox/src/target_process.h +++ b/sandbox/src/target_process.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -8,6 +8,9 @@ #include <windows.h> #include "base/basictypes.h" +#include "base/memory/scoped_ptr.h" +#include "base/win/scoped_handle.h" +#include "base/win/scoped_process_information.h" #include "sandbox/src/crosscall_server.h" #include "sandbox/src/sandbox_types.h" @@ -33,8 +36,10 @@ class TargetProcess { void Release() {} // Creates the new target process. The process is created suspended. - DWORD Create(const wchar_t* exe_path, const wchar_t* command_line, - const wchar_t* desktop, PROCESS_INFORMATION* target_info); + DWORD Create(const wchar_t* exe_path, + const wchar_t* command_line, + const wchar_t* desktop, + base::win::ScopedProcessInformation* target_info); // Destroys the target process. void Terminate(); @@ -46,7 +51,7 @@ class TargetProcess { // Returns the handle to the target process. HANDLE Process() const { - return sandbox_process_; + return sandbox_process_info_.process_handle(); } // Returns the handle to the job object that the target process belongs to. @@ -62,47 +67,43 @@ class TargetProcess { // Returns the name of the executable. const wchar_t* Name() const { - return exe_name_; + return exe_name_.get(); } // Returns the process id. DWORD ProcessId() const { - return sandbox_process_id_; + return sandbox_process_info_.process_id(); } // Returns the handle to the main thread. HANDLE MainThread() const { - return sandbox_thread_; + return sandbox_process_info_.thread_handle(); } // Transfers a 32-bit variable between the broker and the target. ResultCode TransferVariable(char* name, void* address, size_t size); private: - // The handle to the target process. - HANDLE sandbox_process_; - // The handle to the main thread. - HANDLE sandbox_thread_; - // The process id of the target process. - DWORD sandbox_process_id_; + // Details of the target process. + base::win::ScopedProcessInformation sandbox_process_info_; // The token associated with the process. It provides the core of the // sbox security. - HANDLE lockdown_token_; + base::win::ScopedHandle lockdown_token_; // The token given to the initial thread so that the target process can // start. It has more powers than the lockdown_token. - HANDLE initial_token_; + base::win::ScopedHandle initial_token_; // Kernel handle to the shared memory used by the IPC server. - HANDLE shared_section_; + base::win::ScopedHandle shared_section_; // Job object containing the target process. HANDLE job_; // Reference to the IPC subsystem. - SharedMemIPCServer* ipc_server_; + scoped_ptr<SharedMemIPCServer> ipc_server_; // Provides the threads used by the IPC. This class does not own this pointer. ThreadProvider* thread_pool_; // Base address of the main executable void* base_address_; // Full name of the target executable. - wchar_t* exe_name_; + scoped_ptr_malloc<wchar_t> exe_name_; // Function used for testing. friend TargetProcess* MakeTestTargetProcess(HANDLE process, |