1869805 - Add diagnostics code in prespawn CIG to investigate utility process LoadLibraryOrCrash failures

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement)

Description

[Yannis Juglaret [:yannis]]

While working on bug 1571516 to investigate the failures in bug 1851889 (intermittent failures where LoadLibraryOrCrash fails with ERROR_INVALID_IMAGE_HASH), I reached the following conclusions despite not landing any patch for the moment:

• DLL signing is just cosmetics unless CIG is active: unless CIG is active, loading a corrupt DLL won't trigger ERROR_INVALID_IMAGE_HASH, even if the DLL is signed there just won't be any check;
• if CIG is active however and we load a DLL that is not signed by Microsoft, even if it is signed by us and not corrupt, we will get ERROR_INVALID_IMAGE_HASH (unless we successfully use a proxy load, which leads us back to the no-check situation);
• tests use prespawn CIG whereas regular Firefox users will most likely be using delayed CIG even in Nightly [1];
• prespawn CIG uses proxy loads for the DLLs involved in bug 1851889, that goes through the broker and there are many paths that could potentially fail in this code, and if any of those fails it would ultimately lead to ERROR_INVALID_IMAGE_HASH.

For all these reasons I am rather convinced that the failures in bug 1851889 are somehow caused by prespawn CIG and therefore I propose to add diagnostics code to validate this hypothesis. Some of the work in bug 1571516 can be recycled to investigate future issues, but does not seem necessary at the moment anymore given what I just detailed.

[1] : This is a bit weird and hard to follow, but currently in Nightly builds here is what decides whether or not we use prespawn CIG:

• prespawn CIG is used if and only if GetPrespawnCigExceptionModules() returns Some;
• GetPrespawnCigExceptionModules() returns Some if and only if sharedSection->GetDependentModules() returns a non-empty Span;
• SharedSection::Layout::GetDependentModules() returns a non-empty Span if and only if mDependentModulePathArrayStart is non-zero;
• mDependentModulePathArrayStart is non-zero if and only if we met an injected dependent module or the dynamic blocklist is non-empty.

So, if the dynamic blocklist in non-empty, we'll be using prespawn CIG in Nightly builds (and tests) even if we did not meet an injected dependent module (and I don't think that was intentional). And the dynamic blocklist in tests is always non-empty since bug 1744362 (but only for tests since bug 1809513).

Comment 1

[Yannis Juglaret [:yannis]]

Attachment: Bug 1869805 - Add diagnostics code in prespawn CIG to investigate utility process LoadLibraryOrCrash failures. r=bobowen

In order to investigate intermittent failures in bug 1851889, this patch
temporarily adds diagnostics code in DEBUG builds around prespawn CIG.
It adds logs on the broker side, and it makes sandboxed processes crash
upon CIG failures with diagnostics data available in the crashing
thread's stack.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f0a797ddc89
Add diagnostics code in prespawn CIG to investigate utility process LoadLibraryOrCrash failures. r=bobowen

Comment 3

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6f0a797ddc89