Make chrome.exe built with ASan/Win work with sandbox enabled

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 <process.h>
 
 #include "base/basictypes.h"
 #include "sandbox/win/src/crosscall_client.h"
@@ skipping 56 matching lines @@
 TargetServicesBase::TargetServicesBase() {
 }
 
 ResultCode TargetServicesBase::Init() {
   process_state_.SetInitCalled();
   return SBOX_ALL_OK;
 }
 
 // Failure here is a breach of security so the process is terminated.
 void TargetServicesBase::LowerToken() {
+#if defined(ADDRESS_SANITIZER)
+  // Bind and leak dbghelp.dll before the token is lowered, otherwise
+  // AddressSanitizer will crash when trying to symbolize a report.
+  // TODO: find a better place to do this?

[Timur Iskhodzhanov, 2015/01/30 16:05:30]

Please advise where to move this code?

[Will Harris, 2015/01/30 17:49:17]

Warmup is usually done specific for each sandboxed process type in <process
type>MainPlatformDelegate::EnableSandbox

e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

but you want this for tests, so maybe it should go in controller.cc
DispatchCall?

[Timur Iskhodzhanov, 2015/02/02 15:38:58]

I actually want it to run always (including chrome.exe on ClusterFuzz) rather
than just for tests.

Do you think it's reasonable to duplicate code between <process
type>MainPlatformDelegate::EnableSandbox?
I'd argue this code should be placed somewhere in the shared sandbox code that
runs always when AddressSanitizer is enabled.  I don't see why this should be
process-specific.

+  if (!LoadLibraryA("dbghelp.dll"))
+    // TODO: If this is a good place, define a new SBOX_FATAL exit code.
+    ::TerminateProcess(::GetCurrentProcess(), 0x1234);

[cpu_(ooo_6.6-7.5), 2015/01/30 20:28:24]

correct, this happens ahead on each client.

[Timur Iskhodzhanov, 2015/02/02 15:38:58]

Can you please clarify this comment?
Did you mean "yes, this is a good place, let's keep it here. please define a new
SBOX_FATAL exit code"?

+#endif
   if (ERROR_SUCCESS !=
       SetProcessIntegrityLevel(g_shared_delayed_integrity_level))
     ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_INTEGRITY);
   process_state_.SetRevertedToSelf();
   // If the client code as called RegOpenKey, advapi32.dll has cached some
   // handles. The following code gets rid of them.
   if (!::RevertToSelf())
     ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_DROPTOKEN);
   if (!FlushCachedRegHandles())
     ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_FLUSHANDLES);
@@ skipping 98 matching lines @@
 ResultCode TargetServicesBase::DuplicateHandle(HANDLE source_handle,
                                                DWORD target_process_id,
                                                HANDLE* target_handle,
                                                DWORD desired_access,
                                                DWORD options) {
   return sandbox::DuplicateHandleProxy(source_handle, target_process_id,
                                        target_handle, desired_access, options);
 }
 
 }  // namespace sandbox