windows.diagnosticServiceModule still causes ACCESS_DENIED for WerFault.exe

[BernhardMarconato]

Describe the bug

Windows App SDK 2.5.1 introduced windows.diagnosticServiceModule (see documentation MicrosoftDocs/winrt-related#412 and bug dotnet/diagnostics#5828).
This should allow self-contained dotnet app crash dumps to contain additional managed information, by
WerFault.exe loading mscordaccore.dll during the dump.

Previously, this caused WerFault.exe to receive ACCESS DENIED, because it tried to load mscordaccore.dll with execute rights from
the MSIX install directory. This is normally not possible as MSIX restricts the execute ACL from DLLs for processes without identity.

But even with the MSIX extension added, WerFault.exe still is not able to load the DLL and fails with access denied. This leads to the crash dump not containing dotnet managed information. Even though the MSIX extension has successfully added the execute flag for administrators to the DLL.

Reason: WerFault.exe runs as a normal user process, not elevated. So the ACL by the MSIX extension is not sufficient to cover this scenario: ACE (A;;0x1200a9;;;BA) would need to be (A;;0x1200a9;;;BU).

Steps to reproduce the bug

1. Create a WinUI 3 MSIX packaged app with following AppxManifest snippet:

<Applications>
<Application>
<Extensions>
        <dsm:Extension Category="windows.diagnosticServiceModule">
          <dsm:DiagnosticServiceModules>
            <dsm:DiagnosticServiceModule File="mscordaccore.dll" />
          </dsm:DiagnosticServiceModules>
        </dsm:Extension>

2. Add some crash to your app, e.g. throw exception on button click:

private void OnButtonClick(object sender, RoutedEventArgs e)
{
    throw new InvalidOperationException("Test crash");
}

3. Build and package as MSIX (running from VS is not enough)
4. Install the MSIX package
5. Run your app
6. Start ProcMon with WerFault.exe filter
7. Click the crash button
8. Wait until the app closed after the crash
9. Stop the ProcMon dump
10. Find the WerFault.exe process trying to load mscordaccore.dll from the app MSIX directory, and fail with ACCESS_DENIED:

C:\WINDOWS\system32\WerFault.exe -u -p 15540 -s 7044
User: PCNAME\myuser
Operation: CreateFile
Result: ACCESS DENIED
Path: C:\Program Files\WindowsApps\appname_1.0.0.1234_x64__publisher\mscordaccore.dll

Desired Access:    Generic Read/Execute
Disposition:    Open
Options:    Synchronous IO Non-Alert, Non-Directory File
Attributes:    n/a
ShareMode:    Read, Delete
AllocationSize:    n/a

11. Verify the crash dump does not contain managed information (e.g. !dse should show the stowed exception with the dotnet callstack and exception string)

Expected behavior

WerFault.exe can load mscordaccore.dll for standard users and the resulting crash dump file contains dotnet information.

Screenshots

No response

NuGet package version

2.5.1

Packaging type

Packaged (MSIX)

Windows version

25H2 (26200.9457)

IDE

No response

Additional context

ACL SDDL C:\Program Files\WindowsApps\appname_1.0.0.1234_x64__publisher\mscordaccore.dll:
"D:PAI(A;;0x1200a9;;;BA)(A;;0x1200a9;;;AC)(A;;FA;;;SY)(A;;0x1200a9;;;S-1-15-2-2)(A;;0x1200a9;;;LS)(A;;0x1200a9;;;NS)(A;;FA;;;S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464)(A;;0x1200a9;;;S-1-15-3-1024-3635283841-2530182609-996808640-1887759898-3848208603-3313616867-983405619-2501854204)(A;;FR;;;BU)(XA;;0x1200a9;;;BU;(Exists WIN://PKG))(XA;;0x1200a9;;;BU;(WIN://SYSAPPID Contains "appname_publisher"))(XA;;0x1200a9;;;BU;(WIN://SYSAPPID Contains "appname2_publisher"))(A;;0x1200a9;;;RC)"

[diversenok]

One solution would be to relax permissions on mscordaccore.dll and grant unpackaged applications (such as WerFault) execute access to it.

It might appear as if denying execute access to packaged files is essential for security, but I believe that's not the case here. Blocking FILE_EXECUTE on a DLL while allowing FILE_READ_DATA doesn't deprive attackers from any essential capabilities and vice versa, allowing FILE_EXECUTE on top of FILE_READ_DATA should not introduce security flaws. LoadLibrary failing to load DLLs without FILE_EXECUTE is a client-side protection anyway. The underlying NtCreateSection + NtMapViewOfSection do not need FILE_EXECUTE, they can work perfectly fine solely on FILE_READ_DATA. In other words, anybody can patch LoadLibrary's logic in their own process to remove the FILE_EXECUTE requirement if they want to. Thus, adding (A;;0x1200a9;;;BU) to the DACL should be safe.

[jborean93]

I believe this would also solve the case of trying to debug PowerShell binary modules when the underlying PowerShell engine is through an MSIX packaged installation. It being admin only would help in some situations but if it was made BU instead as proposed here would allow non-admins to debug their .NET written PowerShell modules. Some background info on this PowerShell/PowerShell#27565 (comment)

[DrusTheAxe]

relax permissions on mscordaccore.dll and grant unpackaged applications (such as WerFault) execute access to it

That was the intent.

Changing BA to BU is likely the right answer. We're discussing the ramifications -- need to make sure it opens up the needed access without breaking otherwise expected behavior.