Fix issue while parsing PE rich header.

YARA was making the assumption that the 12 bytes following the `DanS` header contained 3 copies of the 32-bits XOR key, but these bytes are actually padding. As these bytes were usually filled with zeroes before applying the XOR key, they appear to contain the key itself after the XOR operation. However, files like `043066108b68b30fc2c475eae8edfafc080be7d451600eaa283d2c750bddbceb` don't contain zeroes in those bytes, and therefore were not satisfying YARA's assumption.

YARA-X was right: https://github.com/VirusTotal/yara-x/blob/6ada059b9e4b328e2341b42304765c5614fc89af/lib/src/modules/pe/parser.rs#L687-L693

Author: plusvic

libyara/include/yara/pe.h

@@ -365,11 +365,12 @@ typedef struct _IMAGE_NT_HEADERS64
 // IMAGE_FIRST_SECTION doesn't need 32/64 versions since the file header is
 // the same either way.
 
-#define IMAGE_FIRST_SECTION(ntheader) \
-  ((PIMAGE_SECTION_HEADER)(                                             \
-      (BYTE*) ntheader + offsetof(IMAGE_NT_HEADERS32, OptionalHeader) + \
-      yr_le16toh(((PIMAGE_NT_HEADERS32)(ntheader))                      \
-                     ->FileHeader.SizeOfOptionalHeader)))
+#define IMAGE_FIRST_SECTION(ntheader)                                      \
+  ((PIMAGE_SECTION_HEADER) ((BYTE*) ntheader +                             \
+                            offsetof(IMAGE_NT_HEADERS32, OptionalHeader) + \
+                            yr_le16toh(                                    \
+                                ((PIMAGE_NT_HEADERS32) (ntheader))         \
+                                    ->FileHeader.SizeOfOptionalHeader)))
 
 // Subsystem Values
 
@@ -727,22 +728,22 @@ typedef struct _IMAGE_SYMBOL_EX
 // MACROS
 
 // Basic Type of  x
-#define BTYPE(x) ((x) &N_BTMASK)
+#define BTYPE(x) ((x) & N_BTMASK)
 
 // Is x a pointer?
 #ifndef ISPTR
-#define ISPTR(x) (((x) &N_TMASK) == (IMAGE_SYM_DTYPE_POINTER << N_BTSHFT))
+#define ISPTR(x) (((x) & N_TMASK) == (IMAGE_SYM_DTYPE_POINTER << N_BTSHFT))
 #endif
 
 // Is x a function?
 #ifndef ISFCN
-#define ISFCN(x) (((x) &N_TMASK) == (IMAGE_SYM_DTYPE_FUNCTION << N_BTSHFT))
+#define ISFCN(x) (((x) & N_TMASK) == (IMAGE_SYM_DTYPE_FUNCTION << N_BTSHFT))
 #endif
 
 // Is x an array?
 
 #ifndef ISARY
-#define ISARY(x) (((x) &N_TMASK) == (IMAGE_SYM_DTYPE_ARRAY << N_BTSHFT))
+#define ISARY(x) (((x) & N_TMASK) == (IMAGE_SYM_DTYPE_ARRAY << N_BTSHFT))
 #endif
 
 // Is x a structure, union, or enumeration TAG?
@@ -755,10 +756,10 @@ typedef struct _IMAGE_SYMBOL_EX
 #ifndef INCREF
 #define INCREF(x)                                                            \
   ((((x) & ~N_BTMASK) << N_TSHIFT) | (IMAGE_SYM_DTYPE_POINTER << N_BTSHFT) | \
-   ((x) &N_BTMASK))
+   ((x) & N_BTMASK))
 #endif
 #ifndef DECREF
-#define DECREF(x) ((((x) >> N_TSHIFT) & ~N_BTMASK) | ((x) &N_BTMASK))
+#define DECREF(x) ((((x) >> N_TSHIFT) & ~N_BTMASK) | ((x) & N_BTMASK))
 #endif
 
 #pragma pack(pop)
@@ -865,9 +866,9 @@ typedef struct _RICH_VERSION_INFO
 typedef struct _RICH_SIGNATURE
 {
   DWORD dans;
-  DWORD key1;
-  DWORD key2;
-  DWORD key3;
+  DWORD padding_1;
+  DWORD padding_2;
+  DWORD padding_3;
   RICH_VERSION_INFO versions[0];
 } RICH_SIGNATURE, *PRICH_SIGNATURE;
 

libyara/modules/pe/pe.c

@@ -204,19 +204,6 @@ static void pe_parse_rich_signature(PE* pe, uint64_t base_address)
   if (rich_signature == NULL)
     return;
 
-  // The three key values must all be equal and the first dword
-  // XORs to "DanS". Then walk the buffer looking for "Rich" which marks the
-  // end. Technically the XOR key should be right after "Rich" but it's not
-  // important.
-
-  if (yr_le32toh(rich_signature->key1) != yr_le32toh(rich_signature->key2) ||
-      yr_le32toh(rich_signature->key2) != yr_le32toh(rich_signature->key3) ||
-      (yr_le32toh(rich_signature->dans) ^ yr_le32toh(rich_signature->key1)) !=
-          RICH_DANS)
-  {
-    return;
-  }
-
   // Multiply by 4 because we are counting in DWORDs.
   rich_len = (rich_ptr - (DWORD*) rich_signature) * 4;
   raw_data = (BYTE*) yr_malloc(rich_len);
@@ -232,9 +219,7 @@ static void pe_parse_rich_signature(PE* pe, uint64_t base_address)
       "rich_signature.offset");
 
   yr_set_integer(rich_len, pe->object, "rich_signature.length");
-
-  yr_set_integer(
-      yr_le32toh(rich_signature->key1), pe->object, "rich_signature.key");
+  yr_set_integer(yr_le32toh(key), pe->object, "rich_signature.key");
 
   clear_data = (BYTE*) yr_malloc(rich_len);
 
@@ -251,7 +236,7 @@ static void pe_parse_rich_signature(PE* pe, uint64_t base_address)
        rich_ptr < (DWORD*) (clear_data + rich_len);
        rich_ptr++)
   {
-    *rich_ptr ^= rich_signature->key1;
+    *rich_ptr ^= key;
   }
 
   yr_set_sized_string(
@@ -383,7 +368,7 @@ static void pe_parse_debug_directory(PE* pe)
       pdb_path_len = strnlen(
           pdb_path, yr_min(available_space(pe, pdb_path), MAX_PATH));
 
-      if (pdb_path_len > 0 && pdb_path_len < MAX_PATH)
+      if (pdb_path_len >= 0 && pdb_path_len < MAX_PATH)
       {
         yr_set_sized_string(pdb_path, pdb_path_len, pe->object, "pdb_path");
         break;