CSRSS lockdown: destroy CSRSS heap

sandbox/win/src/target_services.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/target_services.h"
 
 #include <new>
 
 #include <process.h>
 #include <stdint.h>
 
 #include "base/win/windows_version.h"
 #include "sandbox/win/src/crosscall_client.h"
 #include "sandbox/win/src/handle_closer_agent.h"
+#include "sandbox/win/src/heap_helper.h"
 #include "sandbox/win/src/ipc_tags.h"
 #include "sandbox/win/src/process_mitigations.h"
 #include "sandbox/win/src/restricted_token_utils.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
 #include "sandbox/win/src/sandbox_types.h"
 #include "sandbox/win/src/sharedmem_ipc_client.h"
 
 namespace {
 
@@ skipping 13 matching lines @@
 // that were made during calls to RegOpenkey and RegOpenKeyEx if it is called
 // with a more restrictive token. Returns true if the flushing is succesful
 // although this behavior is undocumented and there is no guarantee that in
 // fact this will happen in future versions of windows.
 bool FlushCachedRegHandles() {
   return (FlushRegKey(HKEY_LOCAL_MACHINE) &&
           FlushRegKey(HKEY_CLASSES_ROOT) &&
           FlushRegKey(HKEY_USERS));
 }
 
+// Cleans up this process if CSRSS will be disconnected, as this disconnection
+// is not supported Windows behavior.
+// Currently, this step requires closing a heap that this shared with csrss.exe.
+// Closing the ALPC Port handle to csrss.exe leaves this heap in an invalid
+// state. This causes problems if anyone enumerates the heap.
+bool CsrssDisconnectCleanup() {
+  PVOID csr_port_heap = sandbox::FindCsrPortHeap();

[Will Harris, 2017/03/22 19:21:49]

are we not already in sandbox namespace. hmm? strange

[Will Harris, 2017/03/22 19:21:49]

implicit cast from HANDLE to PVOID - is this what you intend?

[liamjm (20p), 2017/04/14 17:27:20]

No. 
changed to HANDLE.

Thanks.

[liamjm (20p), 2017/04/14 17:27:20]

Yeah... 
Just in an unnamed namespace at this point.

We could move a couple of functions into the namespace, if desired.

[Will Harris, 2017/05/01 18:33:15]

yes it seems to make sense to move all these functions to the sandbox anonymous
namespace, and then remove the sandbox:: parts below

+  if (nullptr == csr_port_heap) {

[Will Harris, 2017/03/22 19:21:49]

!csr_port_heap

[liamjm (20p), 2017/04/14 17:27:20]

Done.

+    LOG(ERROR) << "Failed to find CSR Port heap handle" return false;

[Will Harris, 2017/03/22 19:21:49]

win\src\target_services.cc(57): error C2143: syntax error: missing ';' before
'return'

[liamjm (20p), 2017/04/14 17:27:20]

Done.

+  }
+  HeapDestroy(csr_port_heap);
+  return true;
+}
+
 // Checks if we have handle entries pending and runs the closer.
 // Updates is_csrss_connected based on which handle types are closed.
 bool CloseOpenHandles(bool* is_csrss_connected) {
   if (sandbox::HandleCloserAgent::NeedsHandlesClosed()) {
     sandbox::HandleCloserAgent handle_closer;
     handle_closer.InitializeHandlesToClose(is_csrss_connected);
+    if (!*is_csrss_connected) {
+      if (!CsrssDisconnectCleanup()) {
+        return false;
+      }
+    }
     if (!handle_closer.CloseHandles())
       return false;
   }
-
   return true;
 }
 
 // GetUserDefaultLocaleName is not available on WIN XP.  So we'll
 // load it on-the-fly.
 const wchar_t kKernel32DllName[] = L"kernel32.dll";
 typedef decltype(GetUserDefaultLocaleName)* GetUserDefaultLocaleNameFunction;
 
 // Warm up language subsystems before the sandbox is turned on.
 // Tested on Win8.1 x64:
@@ skipping 170 matching lines @@
 void ProcessState::SetRevertedToSelf() {
   if (process_state_ < 3)
     process_state_ = 3;
 }
 
 void ProcessState::SetCsrssConnected(bool csrss_connected) {
   csrss_connected_ = csrss_connected;
 }
 
 }  // namespace sandbox