feat: handle SIGBUS / SIGSEGV only when originating from YARA's memor…

…y blocks (#2020)

* feat: prevent race conditions on signal handler

If multiple threads were running in parallel (e.g. each using a
YR_SCANNER), race conditions could cause the original signal handler
to not be restored correctly.
Add a mutex that controls access to the (process wide) signal handler
to ensure that the YARA signal handler is only installed once and
only removed when it is no longer needed.

* feat: forward signal to old signal handler

If a signal is received that does not originate from YARA (which is
quite possible in multi-threaded applications using YARA), it is
currently ignored. Especially for SIGSEGV, this may cause unexpected
behaviour in the caller, even in threads that do not use YARA.

Add logic to call the original signal handler to handle this case
more cleanly. It is not perfect - with only a process wide signal
handler, it's probably impossible to have a perfect solution -
but it's better than the current behaviour.

* fix: make exception test more verbose

* fix: only handle signals in memory block data

Some callbacks from YARA code are done from YR_TRYCATCH. Currently,
YARA assumes that SIGBUS / SIGSEGV from these callbacks are due to
the scanned memory range, but this is not necessarily true.
Add logic to identify the currently scanned memory block and only
catch the signal if the SIGBUS / SIGSEGV originated from it.

* chore: simplify signal handler forwarding

* feat: reduce caught signals to required for known OS

libyara/exception.h

@@ -33,6 +33,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assert.h>
 #include <yara/globals.h>
 
+typedef struct {
+  void* memfault_from;
+  void* memfault_to;
+  void* jump_back;
+} jumpinfo;
+
+
 #if _WIN32 || __CYGWIN__
 
 #include <windows.h>
@@ -83,65 +90,134 @@ static LONG CALLBACK exception_handler(PEXCEPTION_POINTERS ExceptionInfo)
 
 static LONG CALLBACK exception_handler(PEXCEPTION_POINTERS ExceptionInfo)
 {
-  jmp_buf* jb_ptr;
+  jumpinfo* jump_info;
 
   switch (ExceptionInfo->ExceptionRecord->ExceptionCode)
   {
   case EXCEPTION_IN_PAGE_ERROR:
-  case EXCEPTION_ACCESS_VIOLATION:
-    jb_ptr =
-        (jmp_buf*) yr_thread_storage_get_value(&yr_trycatch_trampoline_tls);
-
-    if (jb_ptr != NULL)
-      longjmp(*jb_ptr, 1);
+    jump_info =
+        (jumpinfo*) yr_thread_storage_get_value(&yr_trycatch_trampoline_tls);
+
+    if (jump_info != NULL)
+    {
+      void* fault_address = (void*) ExceptionInfo->ExceptionRecord->ExceptionInformation[1];
+      if (jump_info->memfault_from <= fault_address && jump_info->memfault_to > fault_address)
+      {
+        longjmp(*(jmp_buf*)jump_info->jump_back, 1);
+      }
+    }
   }
 
   return EXCEPTION_CONTINUE_SEARCH;
 }
 
-#define YR_TRYCATCH(_do_, _try_clause_, _catch_clause_)               \
-  do                                                                  \
-  {                                                                   \
-    if (_do_)                                                         \
-    {                                                                 \
-      jmp_buf jb;                                                     \
-      /* Store pointer to sigjmp_buf in TLS */                        \
-      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, &jb);  \
-      HANDLE exh = AddVectoredExceptionHandler(1, exception_handler); \
-      if (setjmp(jb) == 0)                                            \
-      {                                                               \
-        _try_clause_                                                  \
-      }                                                               \
-      else                                                            \
-      {                                                               \
-        _catch_clause_                                                \
-      }                                                               \
-      RemoveVectoredExceptionHandler(exh);                            \
-      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, NULL); \
-    }                                                                 \
-    else                                                              \
-    {                                                                 \
-      _try_clause_                                                    \
-    }                                                                 \
+#define YR_TRYCATCH(_do_, _try_clause_, _catch_clause_)                      \
+  do                                                                         \
+  {                                                                          \
+    if (_do_)                                                                \
+    {                                                                        \
+      jumpinfo jump_info;                                                    \
+      jump_info.memfault_from = 0;                                           \
+      jump_info.memfault_to = 0;                                             \
+      jmp_buf jb;                                                            \
+      jump_info.jump_back = (void*) &jb;                                     \
+      /* Store pointer to sigjmp_buf in TLS */                               \
+      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, &jump_info);  \
+      HANDLE exh = AddVectoredExceptionHandler(1, exception_handler);        \
+      if (setjmp(jb) == 0)                                                   \
+      {                                                                      \
+        _try_clause_                                                         \
+      }                                                                      \
+      else                                                                   \
+      {                                                                      \
+        _catch_clause_                                                       \
+      }                                                                      \
+      RemoveVectoredExceptionHandler(exh);                                   \
+      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, NULL);        \
+    }                                                                        \
+    else                                                                     \
+    {                                                                        \
+      _try_clause_                                                           \
+    }                                                                        \
   } while (0)
 
 #endif
 
 #else
 
+#if defined(__APPLE__) || defined(__linux__) || defined(_AIX)
+#define CATCH_SIGSEGV 0
+#define CATCH_SIGBUS 1
+#elif defined(BSD)
+// According to #551, older BSD versions use SIGSEGV for invalid mmap access.
+// Newer versions, however, use SIGBUS (tested with FreeBSD 13.2 / OpenBSD 7.4).
+// To be compatible with both, catch SIGBUS and SIGSEGV.
+#define CATCH_SIGSEGV 1
+#define CATCH_SIGBUS 1
+#else // For unknown systems, play it safe by catching both
+#define CATCH_SIGSEGV 1
+#define CATCH_SIGBUS 1
+#endif
+
+#include <pthread.h>
 #include <setjmp.h>
 #include <signal.h>
 #include <yara/globals.h>
 
-static void exception_handler(int sig)
+static void exception_handler(int sig, siginfo_t * info, void *context)
 {
-  if (sig == SIGBUS || sig == SIGSEGV)
+  if (sig != SIGBUS && sig != SIGSEGV)
+  {
+    return;
+  }
+  jumpinfo* jump_info = (jumpinfo*) yr_thread_storage_get_value(&yr_trycatch_trampoline_tls);
+
+  if (jump_info != NULL)
   {
-    jmp_buf* jb_ptr =
-        (jmp_buf*) yr_thread_storage_get_value(&yr_trycatch_trampoline_tls);
+    void* fault_address = (void*) info->si_addr;
+    if (jump_info->memfault_from <= fault_address && jump_info->memfault_to > fault_address)
+    {
+      siglongjmp(*(sigjmp_buf*)jump_info->jump_back, 1);
+    }
+  }
+
+  // If we're here, the signal we received didn't originate from YARA.
+  // In this case, we want to invoke the original signal handler, which may handle the signal.
 
-    if (jb_ptr != NULL)
-      siglongjmp(*jb_ptr, 1);
+  // Lock the exception handler mutex to prevent simultaneous write access while we read the old signal handler
+  pthread_mutex_lock(&exception_handler_mutex);
+  struct sigaction old_handler;
+  if (sig == SIGBUS)
+    old_handler = old_sigbus_exception_handler;
+  else
+    old_handler = old_sigsegv_exception_handler;
+  pthread_mutex_unlock(&exception_handler_mutex);
+
+  if (old_handler.sa_flags & SA_SIGINFO)
+  {
+    old_handler.sa_sigaction(sig, info, context);
+  }
+  else if (old_handler.sa_handler == SIG_DFL)
+  {
+    // Old handler is the default action. To do this, set the signal handler back to default and raise the signal.
+    // This is fairly volatile - since this is not an atomic operation, signals from other threads might also
+    // cause the default action while we're doing this. However, the default action will typically cause a
+    // process termination anyway.
+    pthread_mutex_lock(&exception_handler_mutex);
+    struct sigaction current_handler;
+    sigaction(sig, &old_handler, &current_handler);
+    raise(sig);
+    sigaction(sig, &current_handler, NULL);
+    pthread_mutex_unlock(&exception_handler_mutex);
+  }
+  else if (old_handler.sa_handler == SIG_IGN)
+  {
+    // SIG_IGN wants us to ignore the signal
+    return;
+  }
+  else
+  {
+    old_handler.sa_handler(sig);
   }
 }
 
@@ -152,18 +228,28 @@ typedef struct sigaction sa;
   {                                                                   \
     if (_do_)                                                         \
     {                                                                 \
-      struct sigaction old_sigbus_act;                                \
-      struct sigaction old_sigsegv_act;                               \
-      struct sigaction act;                                           \
+      pthread_mutex_lock(&exception_handler_mutex);                   \
+      if (exception_handler_usecount == 0)                            \
+      {                                                               \
+        struct sigaction act;                                         \
+        /* Set exception handler for SIGSEGV / SIGBUS */              \
+        act.sa_sigaction = exception_handler;                         \
+        act.sa_flags = SA_SIGINFO | SA_ONSTACK;                       \
+        sigfillset(&act.sa_mask);                                     \
+        if (CATCH_SIGBUS)                                             \
+          sigaction(SIGBUS, &act, &old_sigbus_exception_handler);     \
+        if (CATCH_SIGSEGV)                                            \
+          sigaction(SIGSEGV, &act, &old_sigsegv_exception_handler);   \
+      }                                                               \
+      exception_handler_usecount++;                                   \
+      pthread_mutex_unlock(&exception_handler_mutex);                 \
+      jumpinfo ji;                                                    \
+      ji.memfault_from = 0;                                           \
+      ji.memfault_to = 0;                                             \
       sigjmp_buf jb;                                                  \
-      /* Store pointer to sigjmp_buf in TLS */                        \
-      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, &jb);  \
-      /* Set exception handler for SIGBUS and SIGSEGV*/               \
-      act.sa_handler = exception_handler;                             \
-      act.sa_flags = 0; /* SA_ONSTACK? */                             \
-      sigfillset(&act.sa_mask);                                       \
-      sigaction(SIGBUS, &act, &old_sigbus_act);                       \
-      sigaction(SIGSEGV, &act, &old_sigsegv_act);                     \
+      ji.jump_back = (void*) &jb;                                     \
+      /* Store pointer to jumpinfo in TLS */                          \
+      yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, &ji);  \
       if (sigsetjmp(jb, 1) == 0)                                      \
       {                                                               \
         _try_clause_                                                  \
@@ -172,9 +258,17 @@ typedef struct sigaction sa;
       {                                                               \
         _catch_clause_                                                \
       }                                                               \
-      /* Stop capturing SIGBUS and SIGSEGV */                         \
-      sigaction(SIGBUS, &old_sigbus_act, NULL);                       \
-      sigaction(SIGSEGV, &old_sigsegv_act, NULL);                     \
+      pthread_mutex_lock(&exception_handler_mutex);                   \
+      exception_handler_usecount--;                                   \
+      if (exception_handler_usecount == 0)                            \
+      {                                                               \
+        /* Stop capturing relevant signals */                         \
+        if (CATCH_SIGBUS)                                             \
+          sigaction(SIGBUS, &old_sigbus_exception_handler, NULL);     \
+        if (CATCH_SIGSEGV)                                            \
+          sigaction(SIGSEGV, &old_sigsegv_exception_handler, NULL);   \
+      }                                                               \
+      pthread_mutex_unlock(&exception_handler_mutex);                 \
       yr_thread_storage_set_value(&yr_trycatch_trampoline_tls, NULL); \
     }                                                                 \
     else                                                              \

libyara/exec.c

@@ -131,7 +131,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
           offset <= block->base + block->size - sizeof(type))     \
       {                                                           \
         type result;                                              \
-        const uint8_t* data = block->fetch_data(block);           \
+        const uint8_t* data = yr_fetch_block_data(block);         \
         if (data == NULL)                                         \
           return YR_UNDEFINED;                                    \
         result = *(type*) (data + offset - block->base);          \

libyara/include/yara/globals.h

@@ -48,6 +48,13 @@ extern YR_THREAD_STORAGE_KEY yr_yyfatal_trampoline_tls;
 // Thread-local storage (TLS) key used by YR_TRYCATCH.
 extern YR_THREAD_STORAGE_KEY yr_trycatch_trampoline_tls;
 
+#if !(_WIN32 || __CYGWIN__)
+extern struct sigaction old_sigsegv_exception_handler;
+extern struct sigaction old_sigbus_exception_handler;
+extern int exception_handler_usecount;
+extern pthread_mutex_t exception_handler_mutex;
+#endif
+
 // When YARA is built with YR_DEBUG_VERBOSITY defined as larger than 0 it can
 // print debug information to stdout.
 #if 0 == YR_DEBUG_VERBOSITY

libyara/include/yara/types.h

@@ -706,6 +706,8 @@ struct YR_MEMORY_BLOCK
   YR_MEMORY_BLOCK_FETCH_DATA_FUNC fetch_data;
 };
 
+YR_API const uint8_t* yr_fetch_block_data(YR_MEMORY_BLOCK* self);
+
 ///////////////////////////////////////////////////////////////////////////////
 // YR_MEMORY_BLOCK_ITERATOR represents an iterator that returns a series of
 // memory blocks to be scanned by yr_scanner_scan_mem_blocks. The iterator have

libyara/libyara.c

@@ -52,6 +52,17 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 YR_THREAD_STORAGE_KEY yr_yyfatal_trampoline_tls;
 YR_THREAD_STORAGE_KEY yr_trycatch_trampoline_tls;
 
+#if !(_WIN32 || __CYGWIN__)
+
+#include <pthread.h>
+#include <signal.h>
+
+struct sigaction old_sigsegv_exception_handler;
+struct sigaction old_sigbus_exception_handler;
+int exception_handler_usecount = 0;
+pthread_mutex_t exception_handler_mutex = PTHREAD_MUTEX_INITIALIZER;
+#endif
+
 static int init_count = 0;
 
 static struct yr_config_var

libyara/modules/dex/dex.c

@@ -1514,7 +1514,7 @@ int module_load(
 
   foreach_memory_block(iterator, block)
   {
-    const uint8_t* block_data = block->fetch_data(block);
+    const uint8_t* block_data = yr_fetch_block_data(block);
 
     if (block_data == NULL)
       continue;

libyara/modules/dotnet/dotnet.c

@@ -3515,7 +3515,7 @@ int module_load(
   {
     PIMAGE_NT_HEADERS32 pe_header;
 
-    block_data = block->fetch_data(block);
+    block_data = yr_fetch_block_data(block);
 
     if (block_data == NULL)
       continue;

libyara/modules/elf/elf.c

@@ -39,6 +39,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <yara/simple_str.h>
 #include <yara/utils.h>
 #include "../crypto.h"
+#include "../exception.h"
 
 #define MODULE_NAME elf
 
@@ -1113,7 +1114,7 @@ int module_load(
 
   foreach_memory_block(iterator, block)
   {
-    const uint8_t* block_data = block->fetch_data(block);
+    const uint8_t* block_data = yr_fetch_block_data(block);
 
     if (block_data == NULL)
       continue;

libyara/modules/hash/hash.c

@@ -326,7 +326,7 @@ define_function(data_md5)
 
     if (offset >= block->base && offset < block->base + block->size)
     {
-      const uint8_t* block_data = block->fetch_data(block);
+      const uint8_t* block_data = yr_fetch_block_data(block);
 
       if (block_data != NULL)
       {
@@ -456,7 +456,7 @@ define_function(data_sha1)
     // if desired block within current block
     if (offset >= block->base && offset < block->base + block->size)
     {
-      const uint8_t* block_data = block->fetch_data(block);
+      const uint8_t* block_data = yr_fetch_block_data(block);
 
       if (block_data != NULL)
       {
@@ -585,7 +585,7 @@ define_function(data_sha256)
     // if desired block within current block
     if (offset >= block->base && offset < block->base + block->size)
     {
-      const uint8_t* block_data = block->fetch_data(block);
+      const uint8_t* block_data = yr_fetch_block_data(block);
 
       if (block_data != NULL)
       {
@@ -674,7 +674,7 @@ define_function(data_checksum32)
   {
     if (offset >= block->base && offset < block->base + block->size)
     {
-      const uint8_t* block_data = block->fetch_data(block);
+      const uint8_t* block_data = yr_fetch_block_data(block);
 
       if (block_data != NULL)
       {
@@ -765,7 +765,7 @@ define_function(data_crc32)
   {
     if (offset >= block->base && offset < block->base + block->size)
     {
-      const uint8_t* block_data = block->fetch_data(block);
+      const uint8_t* block_data = yr_fetch_block_data(block);
 
       if (block_data != NULL)
       {

libyara/modules/macho/macho.c

@@ -1363,7 +1363,7 @@ int module_load(
 
   foreach_memory_block(iterator, block)
   {
-    const uint8_t* block_data = block->fetch_data(block);
+    const uint8_t* block_data = yr_fetch_block_data(block);
 
     if (block_data == NULL || block->size < 4)
       continue;