Sandbox: Remove raw handles from PolicyBase.

sandbox/win/src/sandbox_policy_base.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/sandbox_policy_base.h"
 
 #include <sddl.h>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/logging.h"
+#include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/win/windows_version.h"
 #include "sandbox/win/src/app_container.h"
 #include "sandbox/win/src/filesystem_dispatcher.h"
 #include "sandbox/win/src/filesystem_policy.h"
 #include "sandbox/win/src/handle_dispatcher.h"
 #include "sandbox/win/src/handle_policy.h"
 #include "sandbox/win/src/job.h"
 #include "sandbox/win/src/interception.h"
 #include "sandbox/win/src/process_mitigations.h"
@@ skipping 437 matching lines @@
   return SBOX_ALL_OK;
 }
 
 ResultCode PolicyBase::AddKernelObjectToClose(const base::char16* handle_type,
                                               const base::char16* handle_name) {
   return handle_closer_.AddHandle(handle_type, handle_name);
 }
 
 void* PolicyBase::AddHandleToShare(HANDLE handle) {
   if (base::win::GetVersion() < base::win::VERSION_VISTA)
-    return NULL;
+    return nullptr;
 
   if (!handle)
-    return NULL;
+    return nullptr;
 
-  HANDLE duped_handle = NULL;
-  ::DuplicateHandle(::GetCurrentProcess(),
-                    handle,
-                    ::GetCurrentProcess(),
-                    &duped_handle,
-                    0,
-                    TRUE,
-                    DUPLICATE_SAME_ACCESS);
-  DCHECK(duped_handle);
-  handles_to_share_.push_back(duped_handle);
+  HANDLE duped_handle = nullptr;
+  if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(),
+                         &duped_handle, 0, TRUE, DUPLICATE_SAME_ACCESS)) {
+    return nullptr;
+  }
+  handles_to_share_.push_back(new base::win::ScopedHandle(duped_handle));

[Will Harris, 2015/07/10 18:05:02]

this is a behavior change, previously we'd push a NULL handle, but it doesn't
seem to actually change any real behavior.

[rvargas (doing something else), 2015/07/10 18:26:45]

Correct, but judging by the (insufficient) dcheck, that was actually a bug
because it was unintended. 

   return duped_handle;
 }
 
-HandleList PolicyBase::GetHandlesBeingShared() {
+const HandleList& PolicyBase::GetHandlesBeingShared() {
   return handles_to_share_;
 }
 
 void PolicyBase::ClearSharedHandles() {
-  for (auto handle : handles_to_share_) {
-    ::CloseHandle(handle);
-  }
-  handles_to_share_.clear();
+  STLDeleteElements(&handles_to_share_);
 }
 
 // When an IPC is ready in any of the targets we get called. We manage an array
 // of IPC dispatchers which are keyed on the IPC tag so we normally delegate
 // to the appropriate dispatcher unless we can handle the IPC call ourselves.
 Dispatcher* PolicyBase::OnMessageReady(IPCParams* ipc,
                                        CallbackGeneric* callback) {
   DCHECK(callback);
   static const IPCParams ping1 = {IPC_PING1_TAG, {UINT32_TYPE}};
   static const IPCParams ping2 = {IPC_PING2_TAG, {INOUTPTR_TYPE}};
@@ skipping 372 matching lines @@
       break;
     }
 
     default: { return SBOX_ERROR_UNSUPPORTED; }
   }
 
   return SBOX_ALL_OK;
 }
 
 }  // namespace sandbox