feat: Update Crashpad and register WER handler (#735)

Crashpad added a new WER (Windows Error Reporting) handler which needs
to be manually registered first in the Windows Registry, and then with
the crashpad client.
This WER module is able to capture a wider range of crashes that would otherwise bypass the in-process structured exception handling (SEH) mechanism.

Co-authored-by: Mischan Toosarani-Hausberger <[email protected]>

.gitignore

@@ -2,12 +2,10 @@
 .DS_Store
 .vs
 CMakeSettings.json
+.idea
 
 # CMake builds
-/build
-/build*
-leak-build
-unit-build
+/*build*
 
 # Run files
 .sentry-native

CHANGELOG.md

@@ -1,24 +1,34 @@
 # Changelog
 
+## Unreleased
+
+**Features**:
+
+- Crashpad on Windows now supports `fast-fail` crashes via a registered Windows Error Reporting (WER) module. ([#735](https://github.com/getsentry/sentry-native/pull/735))
+
+**Internal**:
+
+- Updated Breakpad and Crashpad backends to 2022-09-14. ([#735](https://github.com/getsentry/sentry-native/pull/735))
+
 ## 0.5.0
 
 **Features**
 
 - Provide `on_crash()` callback to allow clients to act on detected crashes.
   Users often inquired about distinguishing between crashes and "normal" events in the `before_send()` hook.
-  `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use 
+  `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use
   `before_send()` only for normal events, while `on_crash()` is only invoked for crashes. This change is backward
   compatible for current users of `before_send()` and allows gradual migration to `on_crash()`
-  ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). 
-  ([#724](https://github.com/getsentry/sentry-native/pull/724), 
-   [#734](https://github.com/getsentry/sentry-native/pull/734))
+  ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)).
+  ([#724](https://github.com/getsentry/sentry-native/pull/724),
+  [#734](https://github.com/getsentry/sentry-native/pull/734))
 
 **Fixes**
 
-- Make Windows ModuleFinder more resilient to missing Debug Info 
+- Make Windows ModuleFinder more resilient to missing Debug Info
   ([#732](https://github.com/getsentry/sentry-native/pull/732))
 - Aligned pre-send event processing in `sentry_capture_event()` with the
-  [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) 
+  [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order)
   ([#729](https://github.com/getsentry/sentry-native/pull/729))
 - Align the default value initialization for the `environment` payload attribute with the
   [developer documentation](https://develop.sentry.dev/sdk/event-payloads/#optional-attribute)

CMakeLists.txt

@@ -436,6 +436,9 @@ if(SENTRY_BACKEND_CRASHPAD)
 			set(CRASHPAD_ENABLE_INSTALL ON CACHE BOOL "Enable crashpad installation" FORCE)
 		endif()
 		add_subdirectory(external/crashpad crashpad_build)
+		if(CRASHPAD_WER_ENABLED)
+			add_compile_definitions(CRASHPAD_WER_ENABLED)
+		endif()
 
 		# set static runtime if enabled
 		if(SENTRY_BUILD_RUNTIMESTATIC AND MSVC)
@@ -448,6 +451,9 @@ if(SENTRY_BACKEND_CRASHPAD)
 			set_property(TARGET crashpad_snapshot PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 			set_property(TARGET crashpad_tools PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 			set_property(TARGET crashpad_util PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
+			if(CRASHPAD_WER_ENABLED)
+				set_property(TARGET crashpad_wer PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
+			endif()
 			set_property(TARGET crashpad_zlib PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 			set_property(TARGET mini_chromium PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
 		endif()
@@ -464,6 +470,9 @@ if(SENTRY_BACKEND_CRASHPAD)
 			set_target_properties(crashpad_util PROPERTIES FOLDER ${SENTRY_FOLDER})
 			set_target_properties(crashpad_zlib PROPERTIES FOLDER ${SENTRY_FOLDER})
 			set_target_properties(mini_chromium PROPERTIES FOLDER ${SENTRY_FOLDER})
+			if(CRASHPAD_WER_ENABLED)
+				set_target_properties(crashpad_wer PROPERTIES FOLDER ${SENTRY_FOLDER})
+			endif()
 		endif()
 
 		target_link_libraries(sentry PRIVATE
@@ -476,9 +485,16 @@ if(SENTRY_BACKEND_CRASHPAD)
 		if(WIN32 AND MSVC)
 			sentry_install(FILES $<TARGET_PDB_FILE:crashpad_handler>
 				DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL)
+			if (CRASHPAD_WER_ENABLED)
+				sentry_install(FILES $<TARGET_PDB_FILE:crashpad_wer>
+					DESTINATION "${CMAKE_INSTALL_BINDIR}" OPTIONAL)
+			endif()
 		endif()
 	endif()
 	add_dependencies(sentry crashpad::handler)
+	if(CRASHPAD_WER_ENABLED)
+		add_dependencies(sentry crashpad::wer)
+	endif()
 elseif(SENTRY_BACKEND_BREAKPAD)
 	option(SENTRY_BREAKPAD_SYSTEM "Use system breakpad" OFF)
 	if(SENTRY_BREAKPAD_SYSTEM)
@@ -568,6 +584,10 @@ if(SENTRY_BUILD_EXAMPLES)
 
 	if(MSVC)
 		target_compile_options(sentry_example PRIVATE $<BUILD_INTERFACE:/wd5105>)
+		if(CRASHPAD_WER_ENABLED)
+			# to test handling SEH by-passing exceptions we need to enable the control flow guard
+			target_compile_options(sentry_example PRIVATE $<BUILD_INTERFACE:/guard:cf>)
+		endif()
 	endif()
 
 	# set static runtime if enabled

CONTRIBUTING.md

@@ -122,7 +122,7 @@ The example can be run manually with a variety of commands to test different
 scenarios. Additionally, it will use the `SENTRY_DSN` env-variable, and can thus
 also be used to capture events/crashes directly to sentry.
 
-The example currently supports the following commends:
+The example currently supports the following commands:
 
 - `capture-event`: Captures an event.
 - `crash`: Triggers a crash to be captured.
@@ -141,6 +141,11 @@ The example currently supports the following commends:
 - `add-stacktrace`: Adds the current thread stacktrace to the captured event.
 - `disable-backend`: Disables the build-configured crash-handler backend.
 - `before-send`: Installs a `before_send()` callback that retains the event.
-- `discarding-before-send`: Installs a `before_send()` callback that retains the event.
+- `discarding-before-send`: Installs a `before_send()` callback that discards the event.
 - `on-crash`: Installs an `on_crash()` callback that retains the crash event. 
 - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event.
+
+Only on Windows using crashpad with its WER handler module: 
+
+- `fastfail`: Crashes the application using the `__fastfail` intrinsic directly, thus by-passing SEH.
+- `stack-buffer-overrun`: Triggers the Windows Control Flow Guard, which also fast fails and in turn by-passes SEH.

README.md

@@ -315,8 +315,14 @@ Other important configuration options include:
 
 - The crashpad backend on macOS currently has no support for notifying the crashing
   process, and can thus not properly terminate sessions or call the registered
-  `before_send` hook. It will also lose any events that have been queued for
+  `before_send` or `on_crash` hook. It will also lose any events that have been queued for
   sending at time of crash.
+- The Crashpad backend on Windows supports fast-fail crashes, which bypass SEH (Structured
+  Exception Handling) primarily for security reasons. `sentry-native` registers a WER (Windows Error
+  Reporting) module, which signals the `crashpad_handler` to send a minidump when a fast-fail crash occurs
+  But since this process bypasses SEH, the application local exception handler is no longer invoked, which
+  also means that for these kinds of crashes, `before_send` and `on_crash` will not be invoked before
+  sending the minidump and thus have no effect.
 
 ## Development
 

examples/example.c

@@ -93,6 +93,53 @@ has_arg(int argc, char **argv, const char *arg)
     return false;
 }
 
+#ifdef CRASHPAD_WER_ENABLED
+int
+call_rffe_many_times()
+{
+    RaiseFailFastException(NULL, NULL, 0);
+    RaiseFailFastException(NULL, NULL, 0);
+    RaiseFailFastException(NULL, NULL, 0);
+    RaiseFailFastException(NULL, NULL, 0);
+    return 1;
+}
+
+typedef int (*crash_func)();
+
+void
+indirect_call(crash_func func)
+{
+    // This code always generates CFG guards.
+    func();
+}
+
+static void
+trigger_stack_buffer_overrun()
+{
+    // Call into the middle of the Crashy function.
+    crash_func func = (crash_func)((uintptr_t)(call_rffe_many_times) + 16);
+    __try {
+        // Generates a STATUS_STACK_BUFFER_OVERRUN exception if CFG triggers.
+        indirect_call(func);
+    } __except (EXCEPTION_EXECUTE_HANDLER) {
+        // CFG fast fail should never be caught.
+        printf(
+            "If you see me, then CFG wasn't enabled (compile with /guard:cf)");
+    }
+    // Should only reach here if CFG is disabled.
+    abort();
+}
+
+static void
+trigger_fastfail_crash()
+{
+    // this bypasses WINDOWS SEH and will only be caught with the crashpad WER
+    // module enabled
+    __fastfail(77);
+}
+
+#endif // CRASHPAD_WER_ENABLED
+
 #ifdef SENTRY_PLATFORM_AIX
 // AIX has a null page mapped to the bottom of memory, which means null derefs
 // don't segfault. try dereferencing the top of memory instead; the top nibble
@@ -250,6 +297,14 @@ main(int argc, char **argv)
     if (has_arg(argc, argv, "crash")) {
         trigger_crash();
     }
+#ifdef CRASHPAD_WER_ENABLED
+    if (has_arg(argc, argv, "fastfail")) {
+        trigger_fastfail_crash();
+    }
+    if (has_arg(argc, argv, "stack-buffer-overrun")) {
+        trigger_stack_buffer_overrun();
+    }
+#endif
     if (has_arg(argc, argv, "assert")) {
         assert(0);
     }

include/sentry.h

@@ -787,6 +787,11 @@ SENTRY_API void sentry_options_set_transport(
  * event, following the cross-SDK session filter order:
  *
  * https://develop.sentry.dev/sdk/sessions/#filter-order
+ *
+ * On Windows the crashpad backend can capture fast-fail crashes which by-pass
+ * SEH. Since the `before_send` is called by a local exception-handler, it will
+ * not be invoked when such a crash happened, even though a minidump will be
+ * sent.
  */
 typedef sentry_value_t (*sentry_event_function_t)(
     sentry_value_t event, void *hint, void *closure);
@@ -841,6 +846,10 @@ SENTRY_API void sentry_options_set_before_send(
  *
  *  - does not work with crashpad on macOS.
  *  - for breakpad on Linux the `uctx` parameter is always NULL.
+ *  - on Windows the crashpad backend can capture fast-fail crashes which
+ * by-pass SEH. Since `on_crash` is called by a local exception-handler, it will
+ * not be invoked when such a crash happened, even though a minidump will be
+ * sent.
  */
 typedef sentry_value_t (*sentry_crash_function_t)(
     const sentry_ucontext_t *uctx, sentry_value_t event, void *closure);

src/backends/sentry_backend_crashpad.cpp

@@ -276,7 +276,6 @@ sentry__crashpad_backend_startup(
 
     base::FilePath database(options->database_path->path);
     base::FilePath handler(absolute_handler_path->path);
-    sentry__path_free(absolute_handler_path);
 
     std::map<std::string, std::string> annotations;
     std::vector<base::FilePath> attachments;
@@ -320,6 +319,42 @@ sentry__crashpad_backend_startup(
         annotations, arguments, /* restartable */ true,
         /* asynchronous_start */ false, attachments);
 
+#ifdef CRASHPAD_WER_ENABLED
+    sentry_path_t *handler_dir = sentry__path_dir(absolute_handler_path);
+    sentry_path_t *wer_path = nullptr;
+    if (handler_dir) {
+        wer_path = sentry__path_join_str(handler_dir, "crashpad_wer.dll");
+        sentry__path_free(handler_dir);
+    }
+
+    if (wer_path && sentry__path_is_file(wer_path)) {
+        SENTRY_TRACEF("registering crashpad WER handler "
+                      "\"%" SENTRY_PATH_PRI "\"",
+            wer_path->path);
+
+        // The WER handler needs to be registered in the registry first.
+        DWORD dwOne = 1;
+        LSTATUS reg_res = RegSetKeyValueW(HKEY_CURRENT_USER,
+            L"Software\\Microsoft\\Windows\\Windows Error Reporting\\"
+            L"RuntimeExceptionHelperModules",
+            wer_path->path, REG_DWORD, &dwOne, sizeof(DWORD));
+        if (reg_res != ERROR_SUCCESS) {
+            SENTRY_WARN("registering crashpad WER handler in registry failed");
+        } else {
+            std::wstring wer_path_string(wer_path->path);
+            if (!client.RegisterWerModule(wer_path_string)) {
+                SENTRY_WARN("registering crashpad WER handler module failed");
+            }
+        }
+
+        sentry__path_free(wer_path);
+    } else {
+        SENTRY_WARN("crashpad WER handler module not found");
+    }
+#endif // CRASHPAD_WER_ENABLED
+
+    sentry__path_free(absolute_handler_path);
+
     if (success) {
         SENTRY_DEBUG("started crashpad client handler");
     } else {

tests/conftest.py

@@ -6,7 +6,7 @@
 
 
 def enumerate_unittests():
-    regexp = re.compile("XX\((.*?)\)")
+    regexp = re.compile(r"XX\((.*?)\)")
     # TODO: actually generate the `tests.inc` file with python
     curdir = os.path.dirname(os.path.realpath(__file__))
     with open(os.path.join(curdir, "unit/tests.inc"), "r") as testsfile:
@@ -28,3 +28,18 @@ def cmake(tmp_path_factory):
     yield cmake.compile
 
     cmake.destroy()
+
+
+def pytest_addoption(parser):
+    parser.addoption(
+        "--with_crashpad_wer",
+        action="store_true",
+        help="Enables tests for the crashpad WER module on Windows",
+    )
+
+
+def pytest_runtest_setup(item):
+    if "with_crashpad_wer" in item.keywords and not item.config.getoption(
+        "--with_crashpad_wer"
+    ):
+        pytest.skip("need --with_crashpad_wer to run this test")