Hrmm.. yes, that will happen. I wondered briefly why this doesn't kill the browser process too since we also hook NtMapViewOfSection there but that's because we install the hook in the browser process a tiny bit later on after the process heaps are initialized.

https://codesearch.chromium.org/chromium/src/sandbox/win/src/target_interceptions.cc?l=37 is the thing that calls RtlCreateHeap and while we could re-jigger this not to (probably by using an image section as scratch space as the original report suggests) it would be a little bit of work because we use the created heap in quite a few places.

+robliao@ was trying out AppVerifier in the not too distant past and might have an opinion how much we'd like to support running Chrome under AppVerifier with the sandbox enabled. 

Oh, and thanks for the detailed report! If you'd like slightly more convenient debugging in the future, Chrome's symbol server is here: https://chromium-browser-symsrv.commondatastorage.googleapis.com

Thanks for the symbol server info! Looks like the heap is used to store the Unicode version of the ANSI strings (PE module name, filename). You could consider changing the code to just using the ANSI strings, which could be pointers to the string data in the mapped PE module / NTDLL Loader data.

Another option is to switch to using stack buffers for the strings [MAX_PATH should be enough for each]. Does anyone hold a pointer to the strings after the agent->OnDllLoad call has returned? (I guess not since the heap buffers are freed immediately before TargetNtMapViewOfSection returns)

I am adding 'RB-Stable' for M60. Please feel free to edit if required.

Thank you!

IIRC, Chrome has always needed --no-sandbox (and occassionally some heap flags) to run under AppVerifier.

If we can get it to run with the sandbox enabled, that would certainly be nice, but I'm not sure how possible that is as we lock down many things in the sandbox.

wfh@ would have better clarity on that.

I'm not sure this qualifies for RB-Stable. Chrome historically has had problems running under AppVerifier out of the box, so I'm clearing the flag.

When testing fixes for this bug, don't forget to turn off App Verifier before typing "git cl web" - otherwise bad things happen.

crrev.com/c/925821 is running try jobs now

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da69db99fc30649947ba1f78b48fc9e6afe0de0b

commit da69db99fc30649947ba1f78b48fc9e6afe0de0b
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Thu Feb 22 01:50:38 2018

Fix Windows sandbox to work with Application Verifier

Application Verifier is a useful tool for tracking down bugs on Windows.
However Chrome's sandbox has not played well with App Verifier - it ends
up initializing a heap before the process is ready for this. This change
teaches the NtMapViewOfSection to skip InitHeap when handling the App
Verifier DLLs, which don't need patching anyway.

With this change I can run Chrome with the default App Verifier settings
enabled with the exception of Handles and TLS (those failures probably
don't represent real bugs), without using --no-sandbox.

Bug:  752344 
Change-Id: Id1fd4be9c0f080513bbdb649ed5cf2637df8474c
Reviewed-on: https://chromium-review.googlesource.com/925821
Reviewed-by: James Forshaw <forshaw@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538307}
[modify] https://crrev.com/da69db99fc30649947ba1f78b48fc9e6afe0de0b/sandbox/win/src/sandbox_nt_util.cc
[modify] https://crrev.com/da69db99fc30649947ba1f78b48fc9e6afe0de0b/sandbox/win/src/sandbox_nt_util.h
[modify] https://crrev.com/da69db99fc30649947ba1f78b48fc9e6afe0de0b/sandbox/win/src/target_interceptions.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1155ea4b7b9dea521fe031cdd6b1856ab21dfd8b

commit 1155ea4b7b9dea521fe031cdd6b1856ab21dfd8b
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Fri Feb 23 19:18:14 2018

Revert "Fix Windows sandbox to work with Application Verifier"

This reverts commit da69db99fc30649947ba1f78b48fc9e6afe0de0b.

Reason for revert: Suspected of causing the crashes tracked by  crbug.com/814641 

Original change's description:
> Fix Windows sandbox to work with Application Verifier
> 
> Application Verifier is a useful tool for tracking down bugs on Windows.
> However Chrome's sandbox has not played well with App Verifier - it ends
> up initializing a heap before the process is ready for this. This change
> teaches the NtMapViewOfSection to skip InitHeap when handling the App
> Verifier DLLs, which don't need patching anyway.
> 
> With this change I can run Chrome with the default App Verifier settings
> enabled with the exception of Handles and TLS (those failures probably
> don't represent real bugs), without using --no-sandbox.
> 
> Bug:  752344 
> Change-Id: Id1fd4be9c0f080513bbdb649ed5cf2637df8474c
> Reviewed-on: https://chromium-review.googlesource.com/925821
> Reviewed-by: James Forshaw <forshaw@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#538307}

TBR=brucedawson@chromium.org,forshaw@chromium.org,wfh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  752344 
Change-Id: Icd8feea1779f2891389762db0773ecf0ed9ad762
Reviewed-on: https://chromium-review.googlesource.com/934862
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538851}
[modify] https://crrev.com/1155ea4b7b9dea521fe031cdd6b1856ab21dfd8b/sandbox/win/src/sandbox_nt_util.cc
[modify] https://crrev.com/1155ea4b7b9dea521fe031cdd6b1856ab21dfd8b/sandbox/win/src/sandbox_nt_util.h
[modify] https://crrev.com/1155ea4b7b9dea521fe031cdd6b1856ab21dfd8b/sandbox/win/src/target_interceptions.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a695cf342559b85f2871a5851172fae9d0af3ae3

commit a695cf342559b85f2871a5851172fae9d0af3ae3
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Mon Apr 23 11:46:21 2018

Fix Windows sandbox to work with Application Verifier

Application Verifier is a useful tool for tracking down bugs on Windows.
However Chrome's sandbox has not played well with App Verifier - it ends
up initializing a heap before the process is ready for this. This change
teaches the NtMapViewOfSection to skip InitHeap when handling the App
Verifier DLLs, which don't need patching anyway.

With this change I can run Chrome with the default App Verifier settings
enabled with the exception of Handles and TLS (those failures probably
don't represent real bugs), without using --no-sandbox.

This is an update to crrev.com/c/925821 with null checks added. The
previous CL was reverted due to crashes on some machines, which
unfortunately were never diagnosed.

Bug:  752344 ,792289, 814641 
Change-Id: I52eadcdbc2c1f3dcc76bddf3b97e903143461757
Reviewed-on: https://chromium-review.googlesource.com/1014666
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552672}
[modify] https://crrev.com/a695cf342559b85f2871a5851172fae9d0af3ae3/sandbox/win/src/sandbox_nt_util.cc
[modify] https://crrev.com/a695cf342559b85f2871a5851172fae9d0af3ae3/sandbox/win/src/sandbox_nt_util.h
[modify] https://crrev.com/a695cf342559b85f2871a5851172fae9d0af3ae3/sandbox/win/src/target_interceptions.cc

Looks like the reland stuck. Closing as fixed.