Replace handles that the handle closer closes with dummy Events.

sandbox/win/src/handle_closer_agent.cc

 // 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 "sandbox/win/src/handle_closer_agent.h"
 
 #include "base/logging.h"
 #include "sandbox/win/src/nt_internals.h"
 #include "sandbox/win/src/win_utils.h"
 
@@ skipping 12 matching lines @@
   NTSTATUS status = STATUS_UNSUCCESSFUL;
   __try {
     status = QueryObject(handle, ObjectTypeInformation, buffer, *size, size);
   } __except(GetExceptionCode() == STATUS_INVALID_HANDLE ?
                  EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
     status = STATUS_INVALID_HANDLE;
   }
   return status;
 }
 
+bool AttemptToStuffHandleSlot(HANDLE to_stuff, const base::string16& type) {

[cpu_(ooo_6.6-7.5), 2015/02/18 18:00:00]

to_stuff  -> closed_handle ?

[Will Harris, 2015/02/18 22:11:28]

Done.

+  // Only attempt to stuff Files and Events at the moment.
+  if (type != L"Event" &&
+      type != L"File") {
+    return true;
+  }
+
+  HANDLE dummy = ::CreateEvent(NULL, FALSE, FALSE, NULL);

[forshaw, 2015/02/18 10:50:10]

Can we just use one event object for all stuffed handles in a process? Each new
event does consume some pool in the kernel for the event object which while not
a massive burden especially on x64 there seems no reason to waste memory
unnecessarily. The memory footprint of a process handle entry is ultimately
lower than an event object. I can't see a security/compatibility risk of doing
this considering what we're already breaking.

[cpu_(ooo_6.6-7.5), 2015/02/18 18:00:00]

given that is single threaded we can indeed use a function static and make it
once.  The just dup dup dup

[Will Harris, 2015/02/18 22:11:29]

we can't create the template handle static in this function because then it will
be likely to be the same handle ID as the one we just closed, so instead I
create it in constructor, and then close it again after we're done with the
handle closer agent.

+
+  if (dummy == INVALID_HANDLE_VALUE)

[forshaw, 2015/02/18 10:50:10]

While unlikely to fail CreateEvent actually returns NULL on error not
INVALID_HANDLE_VALUE.

[Will Harris, 2015/02/18 22:11:29]

Done.

+    return false;
+
+  std::vector<HANDLE> to_close;
+  DWORD options = DUPLICATE_SAME_ACCESS;

[Sigurður Ásgeirsson, 2015/02/18 13:56:00]

looks pretty reasonable - you may want to document the assumptions this relies
on, namely what the handle reuse policy is, and the reliance on
single-threadedness for this to work reliably.

[Will Harris, 2015/02/18 22:11:29]

changed this to 0 access as per other comments.

+
+  while (reinterpret_cast<uintptr_t>(dummy) <
+         reinterpret_cast<uintptr_t>(to_stuff)) {
+    HANDLE dup_dummy;

[cpu_(ooo_6.6-7.5), 2015/02/18 18:00:00]

move dup_dummy below 51

[Will Harris, 2015/02/18 22:11:29]

Acknowledged.

+    to_close.push_back(dummy);
+
+    if (!::DuplicateHandle(::GetCurrentProcess(), dummy, ::GetCurrentProcess(),
+                           &dup_dummy, 0, false, options))
+      break;

[Sigurður Ásgeirsson, 2015/02/18 19:02:47]

come to think of it, another check you may want to add here is that you're
making forward progress - e.g. you expect dup_dummy > dummy.
That way you won't be caught infinitely looping, no matter what...

[Will Harris, 2015/02/18 22:11:28]

We can't do this because we don't know if the handle we are trying to close
might have been created before the handle closer initialized the dummy template
handle, and thus already be lower.

In experiments I've found that the first DuplicateHandle always dups into the
target handle ID.

+    dummy = dup_dummy;
+  }
+
+  if (dummy != to_stuff)
+    to_close.push_back(dummy);
+
+  for (auto h : to_close)
+    ::CloseHandle(h);
+
+  // We want to know when we're not able to stuff handles.
+  DCHECK(dummy == to_stuff);
+
+  return dummy == to_stuff;
+}
+
 }  // namespace
 
 namespace sandbox {
 
 // Memory buffer mapped from the parent, with the list of handles.
 SANDBOX_INTERCEPT HandleCloserInfo* g_handles_to_close = NULL;
 
 bool HandleCloserAgent::NeedsHandlesClosed() {
   return g_handles_to_close != NULL;
 }
@@ skipping 86 matching lines @@
       if (!names.empty()) {
         // Move on to the next handle if this name doesn't match.
         if (!GetHandleName(handle, &handle_name) || !names.count(handle_name))
           continue;
       }
 
       if (!::SetHandleInformation(handle, HANDLE_FLAG_PROTECT_FROM_CLOSE, 0))
         return false;
       if (!::CloseHandle(handle))
         return false;
+      // Attempt to stuff this handle with a new empty Event.

[cpu_(ooo_6.6-7.5), 2015/02/18 18:00:00]

"new dummy event"

[Will Harris, 2015/02/18 22:11:29]

Done.

+      AttemptToStuffHandleSlot(handle, result->first);
     }
   }
 
   return true;
 }
 
 }  // namespace sandbox