And MOZ_ASSERT_UNREACHABLE with MOZ_CRASH.
MOZ_ASSERT and MOZ_ASSERT_UNREACHABLE are only checked in debug builds, so release builds' tests are not checking these assertions (though these tests could still have failed later with less-obvious errors).
Depends on D207377
Differential Revision: https://phabricator.services.mozilla.com/D207378
Windows has an ill-documented subsystem known as the Application Launch
Prefetcher, which apparently preloads DLLs for a binary based on what
DLLs that binary loaded last time.
For Firefox, that's not a great heuristic; we relaunch ourselves as a
subprocess with potentially completely different DLL settings. We're not
the only such application, though, and Windows _does_ have a way to
signal the ALPF to load different DLL sets depending on launch context.
Differential Revision: https://phabricator.services.mozilla.com/D202275
This changes comes with several different refactorings all rolled into one,
unfotunately I couldn't find a way to pull them apart:
- First of all annotations now can either recorded (that is, we copy the value
and have the crash reporting code own the copy) or registered. Several
annotations are changed to use this functionality so that we don't need to
update them as their value change.
- The code in the exception handler is modified to read the annotations from
the mozannotation_client crate. This has the unfortunate side-effect that
we need three different bits of code to serialize them: one for annotations
read from a child process, one for reading annotations from the main process
outside of the exception handler and one for reading annotations from the
main process within the exception handler. As we move to fully
out-of-process crash reporting the last two methods will go away.
- The mozannotation_client crate now doesn't record annotation types anymore.
I realized as I was working on this that storing types at runtime has two
issues: the first one is that buggy code might change the type of an
annotation (that is record it under two different types at two different
moments), the second issue is that types might become corrupt during a
crash, so better enforce them at annotation-writing time. The end result is
that the mozannotation_* crates now only store byte buffers, track the
format the data is stored in (null-terminated string, fixed size buffer,
etc...) but not the type of data each annotation is supposed to contain.
- Which brings us to the next change: concrete types for annotations are now
enforced when they're written out. If an annotation doesn't match the
expected type it's skipped. Storing an annotation with the wrong type will
also trigger an assertion in debug builds.
Differential Revision: https://phabricator.services.mozilla.com/D195248
Windows has an ill-documented subsystem known as the Application Launch
Prefetcher, which apparently preloads DLLs for a binary based on what
DLLs that binary loaded last time.
For Firefox, that's not a great heuristic; we relaunch ourselves as a
subprocess with potentially completely different DLL settings. We're not
the only such application, though, and Windows _does_ have a way to
signal the ALPF to load different DLL sets depending on launch context.
Differential Revision: https://phabricator.services.mozilla.com/D202275
We currently fail to guarantee that OnEndDllLoad is called on the same
gLoaderObserver as OnBeginDllLoad. We must implement additional
synchronization to prevent a race condition where a call to
LoaderPrivateAPIImp::SetObserver would come in between the two and
change gLoaderObserver.
This has led to issues when using MOZ_PROFILER_STARTUP=1 where we would
have sStackWalkSuppressions reach (size_t)-1 instead of 0, later
resulting in deadlock or missing stacks. See bug 1687510 comment 10 for
extra details.
Depends on D181436
Differential Revision: https://phabricator.services.mozilla.com/D181437
Third-party products can start threads in our main process, which can load DLLs before the main thread has gone past SharedSection::ConvertToReadOnly. This patch therefore protects the use of the shared section within patched_NtMapViewOfSection, to guarantee that there is no race condition where the shared section could get converted while being used.
Differential Revision: https://phabricator.services.mozilla.com/D187913
NtMapViewOfSection doesn't write to its out-pointer arguments if it
returns an error. Mimic its behavior a little more closely by ensuring
that whatever values were present are restored if we unmap and abort.
Avoid taking any pointers to the stack while doing so.
Differential Revision: https://phabricator.services.mozilla.com/D187737
Our blocklist code must allow loading blocked modules using
LoadLibraryExW with LOAD_LIBRARY_AS_IMAGE_RESOURCE, so that we can
collect information about them when we want to send untrusted module
pings. This means that we need a trustworthy way to distinguish between
these loads and regular DLL loads.
Currently, we do the distinction by looking at the AllocationProtect
field for the virtual memory mapped for the view. This solution was
introduced with bug 1702717, but unfortunately it doesn't work with
Windows 7. This - mixed with other reasons - has resulted in the crash
spike in bug 1842368.
We should thus move to a more trustworthy solution to distinguish
between these two kinds of DLL loads. The new solution is to instead
check whether the permission to map executable views was asked when the
section that we are mapping was created. Because this solution is past
proof, it also has more chances to be future proof.
Differential Revision: https://phabricator.services.mozilla.com/D183530
With bug 1832467 we have updated our Windows SDK version to 10.0.19041.
As a result, we now have a .retplne section in xul.dll, starting with
Firefox 115. This is a section with PAGE_NOACCESS protection, so
accessing it crashes the process.
Some injected DLLs read the whole memory space dedicated to the xul.dll
image to search for patterns in it. When they hit the .retplne section,
we will crash. This happened for a legit product in bug 1837242, but
also for a malicious DLL in bug 1841751. This is a startup crash.
This changeset blocks the variants of this malicious DLL we know, to
eliminate the associated startup crash spike. Because the DLL does not
use a fixed name, we block by matching on the combination of version
number + timestamp + image size, based on the values found in crash
reports. We additionnally check for a checksum of 0 and the absence of
debug information, both of which are uncommon for legit production-ready
DLLs; this thus helps further reduce the chances of collision.
Differential Revision: https://phabricator.services.mozilla.com/D183096
The previous attempt at avoiding the crash from bug 1733532 resulted in
breaking DLL blocklist code on older versions of Windows, in particular
Windows 7, which led to a crash spike (see bug 1837242).
The detect executable DLL mappings, we must not look at the protection
asked in the arguments of NtMapViewOfSection, but rather at the
protection that was used to create the section. We already do this
through NtQueryVirtualMemory, but in a helper function rather than
directly in patched_NtMapViewOfSection.
Because the helper function does not have MOZ_NO_STACK_PROTECTOR, it
does not avoid stack cookie checks when NtMapViewOfSection is called
from Thread32Next. To mitigate the crash from bug 1733532, we need to
move the call to NtQueryVirtualMemory to the main patched function, at
the (reasonable) cost of losing the stack cookie check on local
variable mbi since this function has MOZ_NO_STACK_PROTECTOR.
Differential Revision: https://phabricator.services.mozilla.com/D182982
While fixing a crash in bug 1733532, we accidentally broke the DLL
blocklist on older versions of Windows (Windows 7, some versions
of Windows 10, and possibly Windows 8 and 8.1). This is currently
preventing us from mitigating crashes with third-party injected DLLs, in
particular the crash incident from bug 1837242. Considering the volumes
involved, let's temporarily reintroduce bug 1733532 to ensure everyone
has a working blocklist, and deal with bug 1733532 later.
Differential Revision: https://phabricator.services.mozilla.com/D182917
Similar to the utility process in bug 1826393, we are seeing
crashes in the plugin process due to certain anti-virus apps such
as Symantec. This causes us to fail to load the Widevine plugin
with ERROR_MOD_NOT_FOUND.
This patch adds support to add blocklist entries for the GMPlugin
process type, and mirrors the entries added for the utility
process. It also adds a test case to verify the blocklist
integration.
Differential Revision: https://phabricator.services.mozilla.com/D176942
Similar to the utility process in bug 1826393, we are seeing
crashes in the plugin process due to certain anti-virus apps such
as Symantec. This causes us to fail to load the Widevine plugin
with ERROR_MOD_NOT_FOUND.
This patch adds support to add blocklist entries for the GMPlugin
process type, and mirrors the entries added for the utility
process. It also adds a test case to verify the blocklist
integration.
Differential Revision: https://phabricator.services.mozilla.com/D176942
Bug 1733532 introduced a debug-only assertion so that we have failing
gtests if we unintentionnally reintroduce stack buffers in
patched_NtMapViewOfFile. This is important to make the patch from bug
1733532 futureproof.
This helped us realize that the function currently gets a stack cookie
check in non-optimized builds. We can prevent this by enforcing the
absence of stack cookie checks in patched_NtMapViewOfFile with a new
function attribute MOZ_NO_STACK_PROTECTOR.
Differential Revision: https://phabricator.services.mozilla.com/D171362
Thread32Next relies on NtMapViewOfSection to map the snapshot that it works with.
We hook NtMapViewOfSection, so calls to Thread32Next reach our patched_NtMapViewOfSection.
With some third-party software, this results in a crash if we use stack buffers (see bug 1733532),
because for some reason the stack cookie check code is not mapped executable. If we can avoid using
stack buffers in that case, then the third-party DLL should get its result from NtMapViewOfSection
without error.
This change thus splits patched_NtMapViewOfSection so that we only use stack buffers when necessary,
i.e. when an executable mapping is asked. Hopefully this can fix bug 1733532.
Differential Revision: https://phabricator.services.mozilla.com/D169450
A few changes here:
- rename IsDependentModule() to IsInjectedDependentModule(), as that's closer to what it does (and confused me when I was reading this code)
- Move the `#if defined(EARLY_BETA_OR_EARLIER)` out to the calling function so that we always correctly calculate whether a module is dependent, and change it to NIGHTLY_BUILD re bug 1806041
- Since we're now always detecting whether a module is dependent, it seems like a good idea if we were going to be blocking it to make it NoOpEntryPoint even if we're not NIGHTLY_BUILD. This will help if a user adds such a DLL to the dynamic blocklist; I think if that were to happen right now Firefox would crash on launch?
Differential Revision: https://phabricator.services.mozilla.com/D167454
A few changes here:
- rename IsDependentModule() to IsInjectedDependentModule(), as that's closer to what it does (and confused me when I was reading this code)
- Move the `#if defined(EARLY_BETA_OR_EARLIER)` out to the calling function so that we always correctly calculate whether a module is dependent, and change it to NIGHTLY_BUILD re bug 1806041
- Since we're now always detecting whether a module is dependent, it seems like a good idea if we were going to be blocking it to make it NoOpEntryPoint even if we're not NIGHTLY_BUILD. This will help if a user adds such a DLL to the dynamic blocklist; I think if that were to happen right now Firefox would crash on launch?
Differential Revision: https://phabricator.services.mozilla.com/D167454
As with the socket process, we can't automated test that the block works in the GPU process, but I manually verified this. I did add an automated test that ensures blocking something in the GPU process doesn't block it in other processes.
Differential Revision: https://phabricator.services.mozilla.com/D167399
As with the socket process, we can't automated test that the block works in the GPU process, but I manually verified this. I did add an automated test that ensures blocking something in the GPU process doesn't block it in other processes.
Differential Revision: https://phabricator.services.mozilla.com/D167399
The comment mostly explains it. I don't understand why this only happens on PGO builds, but it's 100% reproducible on try builds and the fix seems reasonable enough.
Depends on D165561
Differential Revision: https://phabricator.services.mozilla.com/D165724
- In sandboxBroker.cpp Be more careful about checking whether GetDependentModules() is returning an empty span to avoid ASAN problems
- In TestCrossProcessWin.cpp, make UniquePtr live as long as the Span that wraps it
- In LauncherRegistryInfo, mingw doesn't allow using `constexpr` with expressions containing '|', so just make flags `const` instead.
Differential Revision: https://phabricator.services.mozilla.com/D165561
Two of the three DLLs that Avast injects into Firefox were not properly being blocked when on the dynamic blocklist because they were being loaded before kernel32.dll, and SharedSection::Layout::Resolve() would fail. For the dynamic blocklist part of things we don't actually need the kernel32 exports, so this change moves them to the end of Resolve() and adds an intermediate state where the dynamic blocklist entries have been loaded but not the kernel32 exports. Now all three DLLs can be blocked correctly when on the dynamic blocklist.
Differential Revision: https://phabricator.services.mozilla.com/D164738
This is a refactoring that makes SharedSection::Reset() do the resolving of Kernel32ExportsSolver. This will allow us in a future patch to let the SharedSection attempt to resolve Kernel32ExportsSolver multiple times, as it will fail if kernel32.dll isn't loaded in the process yet, but we still want to initialize the dynamic blocklist in that case.
Differential Revision: https://phabricator.services.mozilla.com/D164486
- AddString should check .Length instead of .MaximumLength
- SharedSection::Init does not have to take PEHeaders
- Improve the boundary check in SharedSection::AddDependentModule
- Use MOZ_LITERAL_UNICODE_STRING in Kernel32ExportsSolver::ResolveInternal
- Use SharedSection::ConvertToReadOnly in TestCrossProcessWin
- Use SharedSection::Reset to close the handle in TestCrossProcessWin
- Typo: s/AddDepenentModule/AddDependentModule/
- TestCrossProcessWin should compare leaf names
Differential Revision: https://phabricator.services.mozilla.com/D164484
Yannis Juglaret
Chris Peterson
Ray Kraesig
Gabriele Svelto
Butkovits Atila
Greg Stoll
Masatoshi Kimura
Cosmin Sabou
Natalia Csoregi
Andrew Osmond
Norisz Fay
serge-sans-paille
Sandor Molnar
Toshihito Kikuchi