diff options
| author | rvargas@chromium.org <rvargas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-27 01:03:43 +0000 |
|---|---|---|
| committer | rvargas@chromium.org <rvargas@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-27 01:03:43 +0000 |
| commit | 166a865e356b1841a1e3bf3b32bc5aaf13215f6e (patch) | |
| tree | 6aeefbef7ebeb5eb1a232b6c88645da240d73a79 /content/common/sandbox_win.cc | |
| parent | 4cb7699e349fabe62f9b4af7894361c4334161f9 (diff) | |
| download | chromium_src-166a865e356b1841a1e3bf3b32bc5aaf13215f6e.zip chromium_src-166a865e356b1841a1e3bf3b32bc5aaf13215f6e.tar.gz chromium_src-166a865e356b1841a1e3bf3b32bc5aaf13215f6e.tar.bz2 | |
Base: Remove Receive() from ScopedHandle.
In general, the OS API contract doesn't guarantee that output variables are
not modified on failure, so a Reeceive pattern is fundamentally insecure.
BUG=318531
TEST=current tests
tbr'ing owners for the consumers.
TBR=jvoung@chromium.org, thakis@chromium.org, sergeyu@chromium.org, grt@chromium.org, gene@chromium.org, youngki@chromium.org
Review URL: https://codereview.chromium.org/71013004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237459 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content/common/sandbox_win.cc')
| -rw-r--r-- | content/common/sandbox_win.cc | 19 |
1 files changed, 11 insertions, 8 deletions
diff --git a/content/common/sandbox_win.cc b/content/common/sandbox_win.cc index 13b3bd4..f5c86f7 100644 --- a/content/common/sandbox_win.cc +++ b/content/common/sandbox_win.cc @@ -467,13 +467,14 @@ BOOL WINAPI DuplicateHandlePatch(HANDLE source_process_handle, if (!::IsProcessInJob(target_process_handle, NULL, &is_in_job)) { // We need a handle with permission to check the job object. if (ERROR_ACCESS_DENIED == ::GetLastError()) { - base::win::ScopedHandle process; + HANDLE temp_handle; CHECK(g_iat_orig_duplicate_handle(::GetCurrentProcess(), target_process_handle, ::GetCurrentProcess(), - process.Receive(), + &temp_handle, PROCESS_QUERY_INFORMATION, FALSE, 0)); + base::win::ScopedHandle process(temp_handle); CHECK(::IsProcessInJob(process, NULL, &is_in_job)); } } @@ -483,10 +484,11 @@ BOOL WINAPI DuplicateHandlePatch(HANDLE source_process_handle, CHECK(!inherit_handle) << kDuplicateHandleWarning; // Duplicate the handle again, to get the final permissions. - base::win::ScopedHandle handle; + HANDLE temp_handle; CHECK(g_iat_orig_duplicate_handle(target_process_handle, *target_handle, - ::GetCurrentProcess(), handle.Receive(), + ::GetCurrentProcess(), &temp_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)); + base::win::ScopedHandle handle(temp_handle); // Callers use CHECK macro to make sure we get the right stack. CheckDuplicateHandle(handle); @@ -600,7 +602,6 @@ base::ProcessHandle StartSandboxedProcess( return process; } - base::win::ScopedProcessInformation target; sandbox::TargetPolicy* policy = g_broker_services->CreatePolicy(); sandbox::MitigationFlags mitigations = sandbox::MITIGATION_HEAP_TERMINATE | @@ -672,11 +673,13 @@ base::ProcessHandle StartSandboxedProcess( TRACE_EVENT_BEGIN_ETW("StartProcessWithAccess::LAUNCHPROCESS", 0, 0); + PROCESS_INFORMATION temp_process_info = {}; result = g_broker_services->SpawnTarget( - cmd_line->GetProgram().value().c_str(), - cmd_line->GetCommandLineString().c_str(), - policy, target.Receive()); + cmd_line->GetProgram().value().c_str(), + cmd_line->GetCommandLineString().c_str(), + policy, &temp_process_info); policy->Release(); + base::win::ScopedProcessInformation target(temp_process_info); TRACE_EVENT_END_ETW("StartProcessWithAccess::LAUNCHPROCESS", 0, 0); |