Fix wrong caching logic in "magic" module.

Closes #1663.

Makefile.am

@@ -138,6 +138,12 @@ test_dotnet_SOURCES = tests/test-dotnet.c tests/util.c
 test_dotnet_LDADD = libyara/.libs/libyara.a
 endif
 
+if MAGIC_MODULE
+check_PROGRAMS+=test-magic
+test_magic_SOURCES = tests/test-magic.c tests/util.c
+test_magic_LDADD = libyara/.libs/libyara.a
+endif
+
 if PB_TESTS_MODULE
 check_PROGRAMS+=test-pb
 test_pb_SOURCES = tests/test-pb.c tests/util.c

libyara/modules.c

@@ -177,9 +177,7 @@ int yr_modules_load(const char* module_name, YR_SCAN_CONTEXT* context)
 
 int yr_modules_unload_all(YR_SCAN_CONTEXT* context)
 {
-  int i;
-
-  for (i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
+  for (int i = 0; i < sizeof(yr_modules_table) / sizeof(YR_MODULE); i++)
   {
     YR_OBJECT* module_structure = (YR_OBJECT*) yr_hash_table_remove(
         context->objects_table, yr_modules_table[i].name, NULL);

libyara/modules/magic/magic.c

@@ -39,11 +39,9 @@ The original idea and inspiration for this module comes from Armin Buescher.
 
 #define MODULE_NAME magic
 
-
 // Thread-local storage key used to store a pointer to a MAGIC_CACHE struct.
 YR_THREAD_STORAGE_KEY magic_tls;
 
-
 typedef struct
 {
   magic_t magic_cookie;
@@ -52,7 +50,6 @@ typedef struct
 
 } MAGIC_CACHE;
 
-
 static int get_cache(MAGIC_CACHE** cache)
 {
   *cache = (MAGIC_CACHE*) yr_thread_storage_get_value(&magic_tls);
@@ -88,7 +85,6 @@ static int get_cache(MAGIC_CACHE** cache)
   return ERROR_SUCCESS;
 }
 
-
 define_function(magic_mime_type)
 {
   YR_SCAN_CONTEXT* context = scan_context();
@@ -111,8 +107,8 @@ define_function(magic_mime_type)
     {
       magic_setflags(cache->magic_cookie, MAGIC_MIME_TYPE);
 
-      cache->cached_mime_type = magic_buffer(
-          cache->magic_cookie, block_data, block->size);
+      cache->cached_mime_type = yr_strdup(
+          magic_buffer(cache->magic_cookie, block_data, block->size));
     }
   }
 
@@ -122,7 +118,6 @@ define_function(magic_mime_type)
   return_string((char*) cache->cached_mime_type);
 }
 
-
 define_function(magic_type)
 {
   MAGIC_CACHE* cache;
@@ -145,8 +140,8 @@ define_function(magic_type)
     {
       magic_setflags(cache->magic_cookie, 0);
 
-      cache->cached_type = magic_buffer(
-          cache->magic_cookie, block_data, block->size);
+      cache->cached_type = yr_strdup(
+          magic_buffer(cache->magic_cookie, block_data, block->size));
     }
   }
 
@@ -161,13 +156,11 @@ begin_declarations
   declare_function("type", "", "s", magic_type);
 end_declarations
 
-
 int module_initialize(YR_MODULE* module)
 {
   return yr_thread_storage_create(&magic_tls);
 }
 
-
 int module_finalize(YR_MODULE* module)
 {
   MAGIC_CACHE* cache = (MAGIC_CACHE*) yr_thread_storage_get_value(&magic_tls);
@@ -181,7 +174,6 @@ int module_finalize(YR_MODULE* module)
   return yr_thread_storage_destroy(&magic_tls);
 }
 
-
 int module_load(
     YR_SCAN_CONTEXT* context,
     YR_OBJECT* module_object,
@@ -191,13 +183,18 @@ int module_load(
   return ERROR_SUCCESS;
 }
 
-
 int module_unload(YR_OBJECT* module)
 {
   MAGIC_CACHE* cache = (MAGIC_CACHE*) yr_thread_storage_get_value(&magic_tls);
 
   if (cache != NULL)
   {
+    if (cache->cached_type != NULL)
+      yr_free((void*) cache->cached_type);
+
+    if (cache->cached_mime_type != NULL)
+      yr_free((void*) cache->cached_mime_type);
+
     cache->cached_type = NULL;
     cache->cached_mime_type = NULL;
   }

tests/test-magic.c

@@ -0,0 +1,44 @@
+#include <stdlib.h>
+#include <unistd.h>
+#include <yara.h>
+
+#include "blob.h"
+#include "util.h"
+
+int main(int argc, char** argv)
+{
+  int result = 0;
+
+  YR_DEBUG_INITIALIZE();
+  YR_DEBUG_FPRINTF(1, stderr, "+ %s() { // in %s\n", __FUNCTION__, argv[0]);
+
+  init_top_srcdir();
+
+  yr_initialize();
+
+  assert_true_rule_blob(
+      "import \"magic\" rule test { condition: \
+      magic.type() contains \"ELF\" }",
+      ELF32_FILE);
+
+  assert_true_rule_blob(
+      "import \"magic\" rule test { condition: \
+      magic.type() contains \"PE32\" and \
+      magic.mime_type() == \"application/x-dosexec\" }",
+      PE32_FILE);
+
+  // Test case for https://github.com/VirusTotal/yara/issues/1663
+  assert_true_rule_blob(
+      "import \"magic\" rule test { condition: \
+      magic.type() contains \"Mach-O\" and \
+      magic.mime_type() == \"application/x-mach-binary\" and \
+      magic.type() contains \"Mach-O\"}",
+      MACHO_X86_FILE);
+
+  yr_finalize();
+
+  YR_DEBUG_FPRINTF(
+      1, stderr, "} = %d // %s() in %s\n", result, __FUNCTION__, argv[0]);
+
+  return result;
+}