New issue
Advanced search Search tips

Issue 834907 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

[Windows sandbox] ASLR mitigations cleanup required.

Project Member Reported by penny...@chromium.org, Apr 19 2018

Issue description

Note to self.

1) Tests: Figure out what behaviour we expect in debug/release, x86/x64, from win7 - latest.  Change the tests to match that.  Note that I may have disabled this test by now -> as the newly set up debug win testing bot is exposing this broken test now.
**Note that applying aslr mitigation in debug build (post-startup) fails with ACCESS_DENIED (which is explicitly ignored in our code for some reason).
**REF old ticket related to ACCESS_DENIED check: crbug/153399


2) Chromium: Figure out what we need to explicitly set as ASLR mitigations in our sandboxing, and on which versions of Windows.
**REF ticket to consider (and close) at same time: crbug/727708


-----------------------------------------------

Did some quick info gathering (for my sanity), dropping it here for now:

(Running hacked binaries for data gathering on my x64, 1607/RS1, corp gWindows machine.)

x64 Release Build

1) Vanilla, NO mitigations set:
Pre-startup: 0101 (5), BU and HE default.
Post-startup: 0101 (5) , BU and HE default.

2) WITH mitigations set:
Pre-startup: 1111 (f), BU, HE, aslr required & no stripped applied.
Post-startup: 1111 (f) , BU, HE, aslr required & no stripped applied.

Questions:
- BU & HE seems to be part of our build settings.  I know /DYNAMICBASE sets up the PE headers to trigger relocation at load... but what is enabling BU/HE by default?  Is it a build setting or an OS version override?
-Why are there old comments in our code about "Can only set ASLR post-startup in debug build"?  Seems to work just fine pre-startup and for release builds.  Any reason you can think of not to turn that back on at least in testing?  At this point, I would expect all 4 permutations of tests to pass: debug/release, pre/post-startup application.

x64 Debug Build

1) Vanilla, NO mitigations set:
Pre-startup: 0101 (5), BU and HE default.
Post-startup: 0101 (5) , BU and HE default.

2) WITH test mitigations set:
Pre-startup: 1111 (f), BU, HE, aslr required & no stripped applied.
Post-startup: 0101 (5) , BU, HE, aslr required & no stripped NOT applied!

Questions:
- Why is post-startup (delayed) actually failing on debug build??  I only noticed this running tests locally (debug build), and this sbox_integration_tests "CheckWin8ASLR" test is actually failing in debug build.  I guess the bots aren't running this anywhere in debug, because everything is green.  NOTE: I debugged this, and found out that ApplyProcessMitigationsToCurrentProcess() is failing to set ASLR with ERROR_ACCESS_DENIED (which is explicitly ignored in the code)!  I still don't understand why we expect this failure sometimes in the wild, despite OLD bug 153399. And why access denied only on debug build???


The state of Chrome usage:
- MITIGATION_RELOCATE_IMAGE and MITIGATION_RELOCATE_IMAGE_REQUIRED (maps to "aslr required" and "no stripped images") are not explicitly  enabled in our code base for any processes.
- MITIGATION_BOTTOM_UP_ASLR is explicitly enabled in our code base for all child processes.
- MITIGATION_HIGH_ENTROPY_ASLR is not explicitly enabled in our code base for any processes.
(But of course I'm seeing BU and HE enabled by default in our binaries.)

Question: is this what we expect to be explicitly set?  Which settings are we hoping/expecting to be set by default or build settings?  I'd like to document the answer very clearly.

 
Owner: ----
Status: Available (was: Assigned)

Sign in to add a comment