Commit b8479b16 authored by Bruce Dawson's avatar Bruce Dawson Committed by Commit Bot

Fix UAF and reenable test

RestrictedTokenTest.OpenLowPrivilegedProcess was occasionally failing
under asan due to a use-after-free failure. Some of the output was
misleading (in particular the line number of the claimed use-after-free)
but the size/offset/alloc-size/alloc-site/free-site information was
enough to find the bug and create a simple fix.

The lesson is to not use a member of an object to identify the object to
delete.

Bug: 1012356
Change-Id: Iac5e032c5f4bc364eb367a8424a18868918f6181
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427055Reviewed-by: default avatarJames Forshaw <forshaw@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810213}
parent a758cf08
......@@ -338,12 +338,13 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) {
::UnregisterWait(tracker->wait_handle);
tracker->wait_handle = INVALID_HANDLE_VALUE;
// Copy process_id so that we can legally reference it even after we have
// found the ProcessTracker object to delete.
const DWORD process_id = tracker->process_id;
// PID is unique until the process handle is closed in dtor.
processes.erase(std::remove_if(processes.begin(), processes.end(),
[&](auto&& p) -> bool {
return p->process_id ==
tracker->process_id;
return p->process_id == process_id;
}),
processes.end());
......
......@@ -143,14 +143,7 @@ SBOX_TESTS_COMMAND int RestrictedTokenTest_IsRestricted(int argc,
return token_groups->GroupCount > 0 ? SBOX_TEST_SUCCEEDED : SBOX_TEST_FAILED;
}
#if defined(ADDRESS_SANITIZER)
// Flaky under asan: https://crbug.com/1012356
#define MAYBE_OpenLowPrivilegedProcess DISABLED_OpenLowPrivilegedProcess
#else
#define MAYBE_OpenLowPrivilegedProcess OpenLowPrivilegedProcess
#endif
TEST(RestrictedTokenTest, MAYBE_OpenLowPrivilegedProcess) {
TEST(RestrictedTokenTest, OpenLowPrivilegedProcess) {
// Test limited privilege to renderer open.
ASSERT_EQ(SBOX_TEST_SUCCEEDED,
RunOpenProcessTest(false, false, GENERIC_READ | GENERIC_WRITE));
......
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