Handle real segfaults in fuzz targets (#602)

This adds handling for real segfaults in native code in our fuzz
targets. Unfortunately, the only way to do that effectively was to have
the C++ code in the fuzzer detect the error, print a message, write the
crash file, and quit rather than returning control to JS so the result
isn't as nice as a normal error but should be more informative than
before.

Author: br-lewis

.gitignore

@@ -53,4 +53,4 @@ node_modules/
 *.tgz
 
 # Dictionaries generated by Jazzer.js
-.JazzerJs-merged-dictionaries
+.JazzerJs-merged-dictionaries

package.json

@@ -60,7 +60,7 @@
 		"packages/*"
 	],
 	"lint-staged": {
-		"**/*": "prettier --write --ignore-unknown --allow-empty --loglevel debug"
+		"**/!(compile_commands.json)*": "prettier --write --ignore-unknown --allow-empty --log-level debug"
 	},
 	"engines": {
 		"node": ">= 14.0.0",

packages/fuzzer/CMakeLists.txt

@@ -154,8 +154,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
     ${BINARY_DIR}/${LIBFUZZER_STATIC_LIB_PATH} -Wl,-no-whole-archive)
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
   target_link_libraries(
-    ${PROJECT_NAME} -Wl,-all_load ${BINARY_DIR}/${LIBFUZZER_STATIC_LIB_PATH}
-    -Wl,-noall_load)
+    ${PROJECT_NAME} -Wl,-all_load ${BINARY_DIR}/${LIBFUZZER_STATIC_LIB_PATH})
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows")
   # Force MSVC to do an MT build, suggested by cmake-js
   cmake_policy(SET CMP0091 NEW)

packages/fuzzer/fuzzing_async.cpp

@@ -13,6 +13,8 @@
 //  limitations under the License.
 
 #include "napi.h"
+#include <csetjmp>
+#include <csignal>
 #include <cstdlib>
 #include <future>
 #include <iostream>
@@ -66,6 +68,15 @@ using FinalizerDataType = void;
 
 TSFN gTSFN;
 
+const std::string SEGFAULT_ERROR_MESSAGE =
+    "Segmentation fault found in fuzz target";
+
+std::jmp_buf errorBuffer;
+
+// See comment on `ErrorSignalHandler` in `fuzzing_sync.cpp` for what this is
+// for
+void ErrorSignalHandler(int signum) { std::longjmp(errorBuffer, signum); }
+
 // The libFuzzer callback when fuzzing asynchronously.
 int FuzzCallbackAsync(const uint8_t *Data, size_t Size) {
   std::promise<void *> promise;
@@ -107,6 +118,14 @@ void CallJsFuzzCallback(Napi::Env env, Napi::Function jsFuzzCallback,
   // thread and continue with the next invocation.
 
   try {
+    // Return point for the segfault error handler
+    // This MUST BE called from the thread that executes the fuzz target (and
+    // thus is the thread with the segfault) otherwise longjmp's behavior is
+    // undefined
+    if (setjmp(errorBuffer) != 0) {
+      std::cerr << SEGFAULT_ERROR_MESSAGE << std::endl;
+      exit(EXIT_FAILURE);
+    }
     if (env != nullptr) {
       auto buffer = Napi::Buffer<uint8_t>::Copy(env, data->data, data->size);
 
@@ -288,6 +307,7 @@ Napi::Value StartFuzzingAsync(const Napi::CallbackInfo &info) {
   context->native_thread = std::thread(
       [](std::vector<std::string> fuzzer_args, AsyncFuzzTargetContext *ctx) {
         try {
+          signal(SIGSEGV, ErrorSignalHandler);
           StartLibFuzzer(fuzzer_args, FuzzCallbackAsync);
         } catch (const JSException &exception) {
         }

packages/fuzzer/fuzzing_sync.cpp

@@ -15,11 +15,16 @@
 #include "fuzzing_sync.h"
 #include "shared/libfuzzer.h"
 #include "utils.h"
+#include <csetjmp>
 #include <csignal>
 #include <cstdlib>
+#include <iostream>
 #include <optional>
 
 namespace {
+const std::string SEGFAULT_ERROR_MESSAGE =
+    "Segmentation fault found in fuzz target";
+
 // Information about a JS fuzz target.
 struct FuzzTargetInfo {
   Napi::Env env;
@@ -36,10 +41,24 @@ std::optional<FuzzTargetInfo> gFuzzTarget;
 // This is only necessary in the sync fuzzing case, as async can be handled
 // much nicer directly in JavaScript.
 volatile std::sig_atomic_t gSignalStatus;
+std::jmp_buf errorBuffer;
 } // namespace
 
 void sigintHandler(int signum) { gSignalStatus = signum; }
 
+// This handles signals that indicate an unrecoverable error (currently only
+// segfaults). Our handling of segfaults is odd because it avoids using our
+// Javascript method to print and instead prints a message within C++ and exits
+// almost immediately. This is because Node seems to really not like being
+// called back into after `longjmp` jumps outside the scope Node thinks it
+// should be in and so things in JS-land get pretty broken. However, catching it
+// here, printing an ok error message, and letting libfuzzer make the crash file
+// is good enough
+void ErrorSignalHandler(int signum) {
+  gSignalStatus = signum;
+  std::longjmp(errorBuffer, signum);
+}
+
 // The libFuzzer callback when fuzzing synchronously
 int FuzzCallbackSync(const uint8_t *Data, size_t Size) {
   // Create a new active scope so that handles for the buffer objects created in
@@ -62,15 +81,24 @@ int FuzzCallbackSync(const uint8_t *Data, size_t Size) {
   // nice for efficiency if we could use a pointer instead of copying.
   //
   auto data = Napi::Buffer<uint8_t>::Copy(gFuzzTarget->env, Data, Size);
-  auto result = gFuzzTarget->target.Call({data});
-
-  if (result.IsPromise()) {
-    AsyncReturnsHandler();
-  } else {
-    SyncReturnsHandler();
+  if (setjmp(errorBuffer) == 0) {
+    auto result = gFuzzTarget->target.Call({data});
+    if (result.IsPromise()) {
+      AsyncReturnsHandler();
+    } else {
+      SyncReturnsHandler();
+    }
   }
 
   if (gSignalStatus != 0) {
+    // if we caught a segfault, print the error message and die, letting
+    // libfuzzer print the crash file. See the comment on `ErrorSignalHandler`
+    // for why
+    if (gSignalStatus == SIGSEGV) {
+      std::cerr << SEGFAULT_ERROR_MESSAGE << std::endl;
+      exit(EXIT_FAILURE);
+    }
+
     // Non-zero exit codes will produce crash files.
     auto exitCode = Napi::Number::New(gFuzzTarget->env, 0);
 
@@ -111,7 +139,7 @@ void StartFuzzing(const Napi::CallbackInfo &info) {
                  info[2].As<Napi::Function>()};
 
   signal(SIGINT, sigintHandler);
-  signal(SIGSEGV, sigintHandler);
+  signal(SIGSEGV, ErrorSignalHandler);
 
   StartLibFuzzer(fuzzer_args, FuzzCallbackSync);
   // Explicitly reset the global function pointer because the JS

tests/signal_handlers/SIGSEGV/fuzz.js

@@ -14,14 +14,18 @@
  * limitations under the License.
  */
 
+const native = require("native-signal");
+
+const RUN_ON_ITERATION = 1000;
+
 let i = 0;
 
 module.exports.SIGSEGV_SYNC = (data) => {
-	if (i === 1000) {
+	if (i === RUN_ON_ITERATION) {
 		console.log("kill with signal");
 		process.kill(process.pid, "SIGSEGV");
 	}
-	if (i > 1000) {
+	if (i > RUN_ON_ITERATION) {
 		console.log("Signal has not stopped the fuzzing process");
 	}
 	i++;
@@ -30,9 +34,26 @@ module.exports.SIGSEGV_SYNC = (data) => {
 module.exports.SIGSEGV_ASYNC = (data) => {
 	// Raising SIGSEGV in async mode does not stop the fuzzer directly,
 	// as the event is handled asynchronously in the event loop.
-	if (i === 1000) {
+	if (i === RUN_ON_ITERATION) {
 		console.log("kill with signal");
 		process.kill(process.pid, "SIGSEGV");
 	}
 	i++;
 };
+
+module.exports.NATIVE_SIGSEGV_SYNC = (data) => {
+	if (i === RUN_ON_ITERATION) {
+		native.sigsegv(0);
+	}
+	if (i > RUN_ON_ITERATION) {
+		console.log("Signal has not stopped the fuzzing process");
+	}
+	i++;
+};
+
+module.exports.NATIVE_SIGSEGV_ASYNC = async (data) => {
+	if (i === RUN_ON_ITERATION) {
+		native.sigsegv(0);
+	}
+	i++;
+};

tests/signal_handlers/SIGSEGV/package.json

@@ -7,7 +7,8 @@
 		"fuzz": "JAZZER_FUZZ=1 jest"
 	},
 	"devDependencies": {
-		"@jazzer.js/jest-runner": "file:../../packages/jest-runner"
+		"@jazzer.js/jest-runner": "file:../../packages/jest-runner",
+		"native-signal": "file:../native-signal"
 	},
 	"jest": {
 		"projects": [

tests/signal_handlers/SIGSEGV/tests.fuzz.js

@@ -14,9 +14,16 @@
  * limitations under the License.
  */
 
-const { SIGSEGV_ASYNC, SIGSEGV_SYNC } = require("./fuzz.js");
+const {
+	SIGSEGV_ASYNC,
+	SIGSEGV_SYNC,
+	NATIVE_SIGSEGV_SYNC,
+	NATIVE_SIGSEGV_ASYNC,
+} = require("./fuzz.js");
 
 describe("Jest", () => {
 	it.fuzz("Sync", SIGSEGV_SYNC);
 	it.fuzz("Async", SIGSEGV_ASYNC);
+	it.fuzz("Native", NATIVE_SIGSEGV_SYNC);
+	it.fuzz("Native Async", NATIVE_SIGSEGV_ASYNC);
 });

tests/signal_handlers/native-signal/CMakeLists.txt

@@ -0,0 +1,45 @@
+cmake_minimum_required(VERSION 3.15)
+cmake_policy(SET CMP0091 NEW)
+cmake_policy(SET CMP0042 NEW)
+
+project(signal_impl)
+
+set(CMAKE_CXX_STANDARD 17) # mostly supported since GCC 7
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(LLVM_ENABLE_LLD TRUE)
+set(CMAKE_POSITION_INDEPENDENT_CODE ON)
+
+# Avoid warning about DOWNLOAD_EXTRACT_TIMESTAMP in CMake 3.24:
+if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.24.0")
+  cmake_policy(SET CMP0135 NEW)
+endif()
+
+# To help with development, let's write compile_commands.json unconditionally.
+set(CMAKE_EXPORT_COMPILE_COMMANDS 1)
+
+include_directories(${CMAKE_JS_INC})
+
+file(GLOB SOURCE_FILES "*.cpp")
+
+add_library(${PROJECT_NAME} SHARED ${SOURCE_FILES} ${CMAKE_JS_SRC})
+set_target_properties(${PROJECT_NAME} PROPERTIES PREFIX "" SUFFIX ".node")
+target_link_libraries(${PROJECT_NAME} ${CMAKE_JS_LIB})
+
+if(MSVC AND CMAKE_JS_NODELIB_DEF AND CMAKE_JS_NODELIB_TARGET)
+  # Generate node.lib
+  execute_process(COMMAND ${CMAKE_AR} /def:${CMAKE_JS_NODELIB_DEF} /out:${CMAKE_JS_NODELIB_TARGET} ${CMAKE_STATIC_LINKER_FLAGS})
+endif()
+
+# Enable the functionality of Node-API version 4 and disable everything added
+# later, so that we don't accidentally break compatibility with older versions
+# of Node (see https://nodejs.org/api/n-api.html#node-api-version-matrix).
+#
+# Note that prebuild recommends in its README to use ${napi_build_version} here,
+# but the variable is only set when cmake-js is invoked via prebuild (in which
+# case the API version is taken from "binary.napi_versions" in package.json).
+# Since we want the build to work in other cases as well, let's just use a
+# constant. (There is currently no point in a dynamic setting anyway since we
+# specify the oldest version that we're compatible with, and Node-API's ABI
+# stability guarantees that this version is available in all future Node-API
+# releases.)
+add_definitions(-DNAPI_VERSION=4)

tests/signal_handlers/native-signal/compile_commands.json

@@ -0,0 +1 @@
+build/compile_commands.json