Skip to content

Commit

Permalink
Fix a stack overflow in the windows sandbox SpawnTarget function.
Browse files Browse the repository at this point in the history
This was caused by my recent change to allow handles other than STDOUT and STDERR to be shared
with the target. Reason for the crash was copying additional handles to the HANDLE array which had
space for 2 handles only.

Fix is to use scoped_ptr instead and allocate appropriate space for all handles being shared.

BUG=486434
R=cpu

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

Cr-Commit-Position: refs/heads/master@{#329276}
  • Loading branch information
ananta authored and Commit bot committed May 11, 2015
1 parent ffea7a5 commit a6ddf97
Showing 1 changed file with 15 additions and 11 deletions.
26 changes: 15 additions & 11 deletions sandbox/win/src/broker_services.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,9 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
// its |lpValue| parameter persist until |DeleteProcThreadAttributeList| is
// called; StartupInformation's destructor makes such a call.
DWORD64 mitigations;
HANDLE inherit_handle_list[2];

std::vector<HANDLE> inherited_handle_list;

base::string16 desktop = policy_base->GetAlternateDesktop();
if (!desktop.empty()) {
startup_info.startup_info()->lpDesktop =
Expand All @@ -434,18 +436,20 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,

HANDLE stdout_handle = policy_base->GetStdoutHandle();
HANDLE stderr_handle = policy_base->GetStderrHandle();
int inherit_handle_count = 0;

if (stdout_handle != INVALID_HANDLE_VALUE)
inherit_handle_list[inherit_handle_count++] = stdout_handle;
inherited_handle_list.push_back(stdout_handle);

// Handles in the list must be unique.
if (stderr_handle != stdout_handle && stderr_handle != INVALID_HANDLE_VALUE)
inherit_handle_list[inherit_handle_count++] = stderr_handle;
inherited_handle_list.push_back(stderr_handle);

HandleList policy_handle_list = policy_base->GetHandlesBeingShared();

HandleList handle_list = policy_base->GetHandlesBeingShared();
for (auto handle : handle_list)
inherit_handle_list[inherit_handle_count++] = handle;
for (auto handle : policy_handle_list)
inherited_handle_list.push_back(handle);

if (inherit_handle_count)
if (inherited_handle_list.size())
++attribute_count;

if (!startup_info.InitializeProcThreadAttributeList(attribute_count))
Expand All @@ -465,11 +469,11 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
}
}

if (inherit_handle_count) {
if (inherited_handle_list.size()) {
if (!startup_info.UpdateProcThreadAttribute(
PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
inherit_handle_list,
sizeof(inherit_handle_list[0]) * inherit_handle_count)) {
&inherited_handle_list[0],
sizeof(HANDLE) * inherited_handle_list.size())) {
return SBOX_ERROR_PROC_THREAD_ATTRIBUTES;
}
startup_info.startup_info()->dwFlags |= STARTF_USESTDHANDLES;
Expand Down

0 comments on commit a6ddf97

Please sign in to comment.