Fix issue where ERROR_TOO_MANY_MATCHES was incorrectly returned.

In some cases ERROR_TOO_MANY_MATCHES was being returned even if CALLBACK_MSG_TOO_MANY_MATCHES says that this error should be ignored. This happened because both _yr_scan_verify_literal_match and _yr_scan_verify_re_match could return ERROR_TOO_MANY_MATCHES by their own when handling chained strings.
VirusTotal · Oct 21, 2021 · 6085d3f · 6085d3f
1 parent f844881
commit 6085d3f
Show file tree

Hide file tree

Showing 2 changed files with 137 additions and 36 deletions.
diff --git a/libyara/scan.c b/libyara/scan.c
@@ -990,25 +990,6 @@ int yr_scan_verify_match(
   if (yr_bitmask_is_set(context->strings_temp_disabled, string->idx))
     return ERROR_SUCCESS;
 
-  if (context->matches[string->idx].count == YR_MAX_STRING_MATCHES)
-  {
-    result = callback(
-        context,
-        CALLBACK_MSG_TOO_MANY_MATCHES,
-        (void*) string,
-        context->user_data);
-
-    if (result == CALLBACK_CONTINUE)
-    {
-      yr_bitmask_set(context->strings_temp_disabled, string->idx);
-      return ERROR_SUCCESS;
-    }
-    else if (result == CALLBACK_ABORT || result == CALLBACK_ERROR)
-      return ERROR_TOO_MANY_MATCHES;
-    else
-      return ERROR_INTERNAL_FATAL_ERROR;
-  }
-
   if (context->flags & SCAN_FLAGS_FAST_MODE && STRING_IS_SINGLE_MATCH(string) &&
       context->matches[string->idx].head != NULL)
     return ERROR_SUCCESS;
@@ -1038,6 +1019,31 @@ int yr_scan_verify_match(
         context, ac_match, data, data_size, data_base, offset);
   }
 
+  // If _yr_scan_verify_literal_match or _yr_scan_verify_re_match return
+  // ERROR_TOO_MANY_MATCHES call the callback with CALLBACK_MSG_TOO_MANY_MATCHES
+  // in order to ask what to do. If the callback returns CALLBACK_CONTINUE
+  // this error is ignored, if not, the error is propagated to the caller.
+  if (result == ERROR_TOO_MANY_MATCHES)
+  {
+    result = callback(
+        context,
+        CALLBACK_MSG_TOO_MANY_MATCHES,
+        (void*) string,
+        context->user_data);
+
+    switch (result)
+    {
+    case CALLBACK_CONTINUE:
+      yr_bitmask_set(context->strings_temp_disabled, string->idx);
+      result = ERROR_SUCCESS;
+      break;
+
+    default:
+      result = ERROR_TOO_MANY_MATCHES;
+      break;
+    }
+  }
+
 #ifdef YR_PROFILING_ENABLED
   if (sample)
   {

diff --git a/tests/test-api.c b/tests/test-api.c
@@ -57,7 +57,6 @@ void test_disabled_rules()
   yr_finalize();
 }
 
-
 const char* _include_callback(
     const char* include_name,
     const char* calling_rule_filename,
@@ -70,7 +69,6 @@ const char* _include_callback(
     return NULL;
 }
 
-
 void test_include_callback()
 {
   YR_COMPILER* compiler = NULL;
@@ -103,7 +101,6 @@ void test_include_callback()
   yr_finalize();
 }
 
-
 void test_file_descriptor()
 {
   YR_COMPILER* compiler = NULL;
@@ -205,7 +202,6 @@ void test_max_string_per_rules()
   yr_finalize();
 }
 
-
 int test_max_match_data_callback(
     YR_SCAN_CONTEXT* context,
     int message,
@@ -272,6 +268,93 @@ void test_max_match_data()
   yr_finalize();
 }
 
+int ignore_too_many_matches(
+    YR_SCAN_CONTEXT* context,
+    int message,
+    void* message_data,
+    void* user_data)
+{
+  return CALLBACK_CONTINUE;
+}
+
+int propagate_too_many_matches(
+    YR_SCAN_CONTEXT* context,
+    int message,
+    void* message_data,
+    void* user_data)
+{
+  if (message == CALLBACK_MSG_TOO_MANY_MATCHES)
+    return CALLBACK_ERROR;
+
+  return CALLBACK_CONTINUE;
+}
+
+void test_too_many_matches()
+{
+  YR_RULES* rules;
+
+  char* rules_str = "\
+      rule t { \
+        strings: \
+          $a = \"aa\" \
+          $b = { 61 61 [-] 61 61} \
+        condition: \
+          any of them \
+       }";
+
+  yr_initialize();
+
+  if (compile_rule(rules_str, &rules) != ERROR_SUCCESS)
+  {
+    perror("compile_rule");
+    exit(EXIT_FAILURE);
+  }
+
+  uint8_t* buffer = (uint8_t*) malloc(2 * YR_MAX_STRING_MATCHES);
+  memset(buffer, 'a', 2 * YR_MAX_STRING_MATCHES);
+
+  int err = yr_rules_scan_mem(
+      rules,
+      (const uint8_t*) buffer,
+      2 * YR_MAX_STRING_MATCHES,
+      0,
+      propagate_too_many_matches,
+      NULL,
+      0);
+
+  if (err != ERROR_TOO_MANY_MATCHES)
+  {
+    fprintf(
+        stderr,
+        "test_too_many_matches failed, expecting ERROR_TOO_MANY_MATCHES, got "
+        "%d\n",
+        err);
+
+    exit(EXIT_FAILURE);
+  }
+
+  err = yr_rules_scan_mem(
+      rules,
+      (const uint8_t*) buffer,
+      2 * YR_MAX_STRING_MATCHES,
+      0,
+      ignore_too_many_matches,
+      NULL,
+      0);
+
+  if (err != ERROR_SUCCESS)
+  {
+    fprintf(
+        stderr,
+        "test_too_many_matches failed, expecting ERROR_SUCCESS, got %d\n",
+        err);
+
+    exit(EXIT_FAILURE);
+  }
+
+  yr_rules_destroy(rules);
+  yr_finalize();
+}
 
 void test_save_load_rules()
 {
@@ -350,7 +433,6 @@ void test_save_load_rules()
   yr_finalize();
 }
 
-
 void test_scanner()
 {
   const char* buf = "dummy";
@@ -385,7 +467,6 @@ void test_scanner()
   yr_compiler_define_boolean_variable(compiler, "bool_var", 0);
   yr_compiler_define_string_variable(compiler, "str_var", "");
 
-
   if (yr_compiler_define_string_variable(compiler, "str_var", "") !=
       ERROR_DUPLICATED_EXTERNAL_VARIABLE)
   {
@@ -574,7 +655,6 @@ void test_issue_834()
   yr_finalize();
 }
 
-
 void ast_callback(
     const YR_RULE* rule,
     const char* string_identifier,
@@ -627,7 +707,6 @@ void test_ast_callback()
   yr_finalize();
 }
 
-
 void stats_for_rules(const char* rules_str, YR_RULES_STATS* stats)
 {
   YR_COMPILER* compiler = NULL;
@@ -662,7 +741,6 @@ void stats_for_rules(const char* rules_str, YR_RULES_STATS* stats)
   yr_finalize();
 }
 
-
 void test_rules_stats()
 {
   YR_RULES_STATS stats;
@@ -749,7 +827,6 @@ void test_rules_stats()
   assert_true_expr(stats.ac_root_match_list_length == 0);
 }
 
-
 void test_issue_920()
 {
   const char* rules_str = "\
@@ -788,8 +865,8 @@ void test_issue_920()
   yr_finalize();
 }
 
-
-void test_runtime_warnings() {
+void test_runtime_warnings()
+{
   // This rule should never match since it will hit the maximum number of
   // matches (see YR_MAX_STRING_MATCHES) and a warning will be issued, and any
   // further matches no longer count.
@@ -810,12 +887,14 @@ void test_runtime_warnings() {
 
   yr_initialize();
 
-  if (yr_compiler_create(&compiler) != ERROR_SUCCESS) {
+  if (yr_compiler_create(&compiler) != ERROR_SUCCESS)
+  {
     perror("yr_compiler_create");
     exit(EXIT_FAILURE);
   }
 
-  if (yr_compiler_add_string(compiler, rules_str, NULL) != ERROR_SUCCESS) {
+  if (yr_compiler_add_string(compiler, rules_str, NULL) != ERROR_SUCCESS)
+  {
     yr_compiler_destroy(compiler);
     perror("yr_compiler_add_string");
     exit(EXIT_FAILURE);
@@ -830,7 +909,14 @@ void test_runtime_warnings() {
 
   yr_compiler_destroy(compiler);
 
-  if (yr_rules_scan_file(rules, prefix_top_srcdir("tests/data/x.txt"), 0, count, &counters, 0) != ERROR_SUCCESS) {
+  if (yr_rules_scan_file(
+          rules,
+          prefix_top_srcdir("tests/data/x.txt"),
+          0,
+          count,
+          &counters,
+          0) != ERROR_SUCCESS)
+  {
     yr_rules_destroy(rules);
     perror("yr_rules_scan_file");
     exit(EXIT_FAILURE);
@@ -848,7 +934,14 @@ void test_runtime_warnings() {
   counters.rules_matching = 0;
   counters.rules_warning = 0;
 
-  if (yr_rules_scan_file(rules, prefix_top_srcdir("tests/data/x.txt"), 0, count, &counters, 0) != ERROR_SUCCESS) {
+  if (yr_rules_scan_file(
+          rules,
+          prefix_top_srcdir("tests/data/x.txt"),
+          0,
+          count,
+          &counters,
+          0) != ERROR_SUCCESS)
+  {
     yr_rules_destroy(rules);
     perror("yr_rules_scan_file");
     exit(EXIT_FAILURE);
@@ -877,6 +970,7 @@ int main(int argc, char** argv)
   test_file_descriptor();
   test_max_string_per_rules();
   test_max_match_data();
+  test_too_many_matches();
   test_include_callback();
   test_save_load_rules();
   test_scanner();
@@ -886,7 +980,8 @@ int main(int argc, char** argv)
   test_issue_920();
   test_runtime_warnings();
 
-  YR_DEBUG_FPRINTF(1, stderr, "} = %d // %s() in %s\n", result, __FUNCTION__, argv[0]);
+  YR_DEBUG_FPRINTF(
+      1, stderr, "} = %d // %s() in %s\n", result, __FUNCTION__, argv[0]);
 
   return result;
 }