Commit 6ab31da1 authored by rvargas's avatar rvargas Committed by Commit bot

Remove implicit HANDLE conversions from sandbox.

BUG=416722
R=wfh@chromium.org

Review URL: https://codereview.chromium.org/606443002

Cr-Commit-Position: refs/heads/master@{#296580}
parent d5dfc9a8
...@@ -457,7 +457,7 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, ...@@ -457,7 +457,7 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
base::win::ScopedProcessInformation process_info; base::win::ScopedProcessInformation process_info;
TargetProcess* target = new TargetProcess(initial_token.Take(), TargetProcess* target = new TargetProcess(initial_token.Take(),
lockdown_token.Take(), lockdown_token.Take(),
job, job.Get(),
thread_pool_); thread_pool_);
DWORD win_result = target->Create(exe_path, command_line, inherit_handles, DWORD win_result = target->Create(exe_path, command_line, inherit_handles,
...@@ -534,8 +534,8 @@ ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) { ...@@ -534,8 +534,8 @@ ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
return SBOX_ERROR_BAD_PARAMS; return SBOX_ERROR_BAD_PARAMS;
if (!::RegisterWaitForSingleObject( if (!::RegisterWaitForSingleObject(
&peer->wait_object, peer->process, RemovePeer, peer.get(), INFINITE, &peer->wait_object, peer->process.Get(), RemovePeer, peer.get(),
WT_EXECUTEONLYONCE | WT_EXECUTEINWAITTHREAD)) { INFINITE, WT_EXECUTEONLYONCE | WT_EXECUTEINWAITTHREAD)) {
peer_map_.erase(peer->id); peer_map_.erase(peer->id);
return SBOX_ERROR_GENERIC; return SBOX_ERROR_GENERIC;
} }
......
...@@ -29,9 +29,9 @@ HANDLE GetMarkerFile(const wchar_t *extension) { ...@@ -29,9 +29,9 @@ HANDLE GetMarkerFile(const wchar_t *extension) {
OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL)); OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL));
CHECK(module.IsValid()); CHECK(module.IsValid());
FILETIME timestamp; FILETIME timestamp;
CHECK(::GetFileTime(module, &timestamp, NULL, NULL)); CHECK(::GetFileTime(module.Get(), &timestamp, NULL, NULL));
marker_path += base::StringPrintf(L"%08x%08x%08x", marker_path += base::StringPrintf(L"%08x%08x%08x",
::GetFileSize(module, NULL), ::GetFileSize(module.Get(), NULL),
timestamp.dwLowDateTime, timestamp.dwLowDateTime,
timestamp.dwHighDateTime); timestamp.dwHighDateTime);
marker_path += extension; marker_path += extension;
...@@ -115,7 +115,7 @@ TEST(HandleCloserTest, CheckForMarkerFiles) { ...@@ -115,7 +115,7 @@ TEST(HandleCloserTest, CheckForMarkerFiles) {
base::string16 handle_name; base::string16 handle_name;
base::win::ScopedHandle marker(GetMarkerFile(kFileExtensions[i])); base::win::ScopedHandle marker(GetMarkerFile(kFileExtensions[i]));
CHECK(marker.IsValid()); CHECK(marker.IsValid());
CHECK(sandbox::GetHandleName(marker, &handle_name)); CHECK(sandbox::GetHandleName(marker.Get(), &handle_name));
command += (L" "); command += (L" ");
command += handle_name; command += handle_name;
} }
...@@ -135,7 +135,7 @@ TEST(HandleCloserTest, CloseMarkerFiles) { ...@@ -135,7 +135,7 @@ TEST(HandleCloserTest, CloseMarkerFiles) {
base::string16 handle_name; base::string16 handle_name;
base::win::ScopedHandle marker(GetMarkerFile(kFileExtensions[i])); base::win::ScopedHandle marker(GetMarkerFile(kFileExtensions[i]));
CHECK(marker.IsValid()); CHECK(marker.IsValid());
CHECK(sandbox::GetHandleName(marker, &handle_name)); CHECK(sandbox::GetHandleName(marker.Get(), &handle_name));
CHECK_EQ(policy->AddKernelObjectToClose(L"File", handle_name.c_str()), CHECK_EQ(policy->AddKernelObjectToClose(L"File", handle_name.c_str()),
SBOX_ALL_OK); SBOX_ALL_OK);
command += (L" "); command += (L" ");
......
...@@ -65,7 +65,7 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc, ...@@ -65,7 +65,7 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc,
reinterpret_cast<OBJECT_TYPE_INFORMATION*>(buffer); reinterpret_cast<OBJECT_TYPE_INFORMATION*>(buffer);
ULONG size = sizeof(buffer) - sizeof(wchar_t); ULONG size = sizeof(buffer) - sizeof(wchar_t);
NTSTATUS error = NTSTATUS error =
QueryObject(handle, ObjectTypeInformation, type_info, size, &size); QueryObject(handle.Get(), ObjectTypeInformation, type_info, size, &size);
if (!NT_SUCCESS(error)) { if (!NT_SUCCESS(error)) {
ipc->return_info.nt_status = error; ipc->return_info.nt_status = error;
return false; return false;
...@@ -79,7 +79,7 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc, ...@@ -79,7 +79,7 @@ bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc,
EvalResult eval = policy_base_->EvalPolicy(IPC_DUPLICATEHANDLEPROXY_TAG, EvalResult eval = policy_base_->EvalPolicy(IPC_DUPLICATEHANDLEPROXY_TAG,
params.GetBase()); params.GetBase());
ipc->return_info.win32_result = ipc->return_info.win32_result =
HandlePolicy::DuplicateHandleProxyAction(eval, handle, HandlePolicy::DuplicateHandleProxyAction(eval, handle.Get(),
target_process_id, target_process_id,
&ipc->return_info.handle, &ipc->return_info.handle,
desired_access, options); desired_access, options);
......
...@@ -39,7 +39,7 @@ SBOX_TESTS_COMMAND int Handle_DuplicateEvent(int argc, wchar_t **argv) { ...@@ -39,7 +39,7 @@ SBOX_TESTS_COMMAND int Handle_DuplicateEvent(int argc, wchar_t **argv) {
HANDLE handle = NULL; HANDLE handle = NULL;
ResultCode result = SandboxFactory::GetTargetServices()->DuplicateHandle( ResultCode result = SandboxFactory::GetTargetServices()->DuplicateHandle(
test_event, target_process_id, &handle, 0, DUPLICATE_SAME_ACCESS); test_event.Get(), target_process_id, &handle, 0, DUPLICATE_SAME_ACCESS);
return (result == SBOX_ALL_OK) ? SBOX_TEST_SUCCEEDED : SBOX_TEST_DENIED; return (result == SBOX_ALL_OK) ? SBOX_TEST_SUCCEEDED : SBOX_TEST_DENIED;
} }
......
...@@ -25,7 +25,7 @@ SBOX_TESTS_COMMAND int Event_Open(int argc, wchar_t **argv) { ...@@ -25,7 +25,7 @@ SBOX_TESTS_COMMAND int Event_Open(int argc, wchar_t **argv) {
desired_access, FALSE, argv[1])); desired_access, FALSE, argv[1]));
DWORD error_open = ::GetLastError(); DWORD error_open = ::GetLastError();
if (event_open.Get()) if (event_open.IsValid())
return SBOX_TEST_SUCCEEDED; return SBOX_TEST_SUCCEEDED;
if (ERROR_ACCESS_DENIED == error_open || if (ERROR_ACCESS_DENIED == error_open ||
...@@ -57,7 +57,7 @@ SBOX_TESTS_COMMAND int Event_CreateOpen(int argc, wchar_t **argv) { ...@@ -57,7 +57,7 @@ SBOX_TESTS_COMMAND int Event_CreateOpen(int argc, wchar_t **argv) {
if (event_name) if (event_name)
event_open.Set(::OpenEvent(EVENT_ALL_ACCESS, FALSE, event_name)); event_open.Set(::OpenEvent(EVENT_ALL_ACCESS, FALSE, event_name));
if (event_create.Get()) { if (event_create.IsValid()) {
DWORD wait = ::WaitForSingleObject(event_create.Get(), 0); DWORD wait = ::WaitForSingleObject(event_create.Get(), 0);
if (initial_state && WAIT_OBJECT_0 != wait) if (initial_state && WAIT_OBJECT_0 != wait)
return SBOX_TEST_FAILED; return SBOX_TEST_FAILED;
...@@ -68,10 +68,11 @@ SBOX_TESTS_COMMAND int Event_CreateOpen(int argc, wchar_t **argv) { ...@@ -68,10 +68,11 @@ SBOX_TESTS_COMMAND int Event_CreateOpen(int argc, wchar_t **argv) {
if (event_name) { if (event_name) {
// Both event_open and event_create have to be valid. // Both event_open and event_create have to be valid.
if (event_open.Get() && event_create) if (event_open.IsValid() && event_create.IsValid())
return SBOX_TEST_SUCCEEDED; return SBOX_TEST_SUCCEEDED;
if (event_open.Get() && !event_create || !event_open.Get() && event_create) if (event_open.IsValid() && !event_create.IsValid() ||
!event_open.IsValid() && event_create.IsValid())
return SBOX_TEST_FAILED; return SBOX_TEST_FAILED;
} else { } else {
// Only event_create has to be valid. // Only event_create has to be valid.
......
...@@ -132,7 +132,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, ...@@ -132,7 +132,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path,
} }
PROCESS_INFORMATION temp_process_info = {}; PROCESS_INFORMATION temp_process_info = {};
if (!::CreateProcessAsUserW(lockdown_token_, if (!::CreateProcessAsUserW(lockdown_token_.Get(),
exe_path, exe_path,
cmd_line.get(), cmd_line.get(),
NULL, // No security attribute. NULL, // No security attribute.
...@@ -164,7 +164,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path, ...@@ -164,7 +164,7 @@ DWORD TargetProcess::Create(const wchar_t* exe_path,
// impersonation token with more rights. This allows the target to start; // impersonation token with more rights. This allows the target to start;
// otherwise it will crash too early for us to help. // otherwise it will crash too early for us to help.
HANDLE temp_thread = process_info.thread_handle(); HANDLE temp_thread = process_info.thread_handle();
if (!::SetThreadToken(&temp_thread, initial_token_)) { if (!::SetThreadToken(&temp_thread, initial_token_.Get())) {
win_result = ::GetLastError(); win_result = ::GetLastError();
// It might be a security breach if we let the target run outside the job // It might be a security breach if we let the target run outside the job
// so kill it before it causes damage. // so kill it before it causes damage.
...@@ -261,13 +261,13 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy, ...@@ -261,13 +261,13 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy,
DWORD access = FILE_MAP_READ | FILE_MAP_WRITE; DWORD access = FILE_MAP_READ | FILE_MAP_WRITE;
HANDLE target_shared_section; HANDLE target_shared_section;
if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_, if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_.Get(),
sandbox_process_info_.process_handle(), sandbox_process_info_.process_handle(),
&target_shared_section, access, FALSE, 0)) { &target_shared_section, access, FALSE, 0)) {
return ::GetLastError(); return ::GetLastError();
} }
void* shared_memory = ::MapViewOfFile(shared_section_, void* shared_memory = ::MapViewOfFile(shared_section_.Get(),
FILE_MAP_WRITE|FILE_MAP_READ, FILE_MAP_WRITE|FILE_MAP_READ,
0, 0, 0); 0, 0, 0);
if (NULL == shared_memory) { if (NULL == shared_memory) {
......
...@@ -129,8 +129,8 @@ TargetPolicy* TestRunner::GetPolicy() { ...@@ -129,8 +129,8 @@ TargetPolicy* TestRunner::GetPolicy() {
} }
TestRunner::~TestRunner() { TestRunner::~TestRunner() {
if (target_process_ && kill_on_destruction_) if (target_process_.IsValid() && kill_on_destruction_)
::TerminateProcess(target_process_, 0); ::TerminateProcess(target_process_.Get(), 0);
if (policy_) if (policy_)
policy_->Release(); policy_->Release();
...@@ -195,8 +195,8 @@ int TestRunner::InternalRunTest(const wchar_t* command) { ...@@ -195,8 +195,8 @@ int TestRunner::InternalRunTest(const wchar_t* command) {
return SBOX_TEST_FAILED_TO_RUN_TEST; return SBOX_TEST_FAILED_TO_RUN_TEST;
// For simplicity TestRunner supports only one process per instance. // For simplicity TestRunner supports only one process per instance.
if (target_process_) { if (target_process_.IsValid()) {
if (IsProcessRunning(target_process_)) if (IsProcessRunning(target_process_.Get()))
return SBOX_TEST_FAILED_TO_RUN_TEST; return SBOX_TEST_FAILED_TO_RUN_TEST;
target_process_.Close(); target_process_.Close();
target_process_id_ = 0; target_process_id_ = 0;
......
...@@ -119,7 +119,7 @@ class TestRunner { ...@@ -119,7 +119,7 @@ class TestRunner {
BrokerServices* broker() { return broker_; } BrokerServices* broker() { return broker_; }
// Returns the process handle for an asynchronous test. // Returns the process handle for an asynchronous test.
HANDLE process() { return target_process_; } HANDLE process() { return target_process_.Get(); }
// Returns the process ID for an asynchronous test. // Returns the process ID for an asynchronous test.
DWORD process_id() { return target_process_id_; } DWORD process_id() { return target_process_id_; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment