Fix issue with warnings. (#1532)

Fix an issue with the TOO_MANY_MATCHES warning where it would set the
STRING_FLAGS_DISABLED bit on the string, but it would never be reset when the
scanner was used to scan another file. This meant that a string that caused
TOO_MANY_MATCHES would be disabled permanently for the lifetime of that scanner,
when the intention was to only disable it for that one scan. A future scan
should have the string re-enabled until it matches too many times again.

I decided to fix this by using a bitmask for each string in the rules. This
allows me to very quickly reset them all without having to touch every string to
unset the STRING_FLAGS_DISABLED flag.

This also returns the STRING_FLAGS_DISABLED flag to it's original meaning, which
was that the string was disabled permanently because it is causing serious
performance problems.

I added a test for all this too.

libyara/include/yara/types.h

@@ -787,6 +787,10 @@ struct YR_SCAN_CONTEXT
   // index N has some global rule that is not satisfied.
   YR_BITMASK* ns_unsatisfied_flags;
 
+  // A bitmap with one bit per string, bit N is set if the string with index
+  // N has too many matches.
+  YR_BITMASK* strings_temp_disabled;
+
   // Array with pointers to lists of matches. Item N in the array has the
   // list of matches for string with index N.
   YR_MATCHES* matches;

libyara/scan.c

@@ -31,6 +31,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <ctype.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <yara/bitmask.h>
 #include <yara/error.h>
 #include <yara/globals.h>
 #include <yara/libyara.h>
@@ -985,7 +986,7 @@ int yr_scan_verify_match(
   if (data_size - offset <= 0)
     return ERROR_SUCCESS;
 
-  if (STRING_IS_DISABLED(string))
+  if (yr_bitmask_is_set(context->strings_temp_disabled, string->idx))
     return ERROR_SUCCESS;
 
   if (context->matches[string->idx].count == YR_MAX_STRING_MATCHES)
@@ -998,7 +999,7 @@ int yr_scan_verify_match(
 
     if (result == CALLBACK_CONTINUE)
     {
-      string->flags |= STRING_FLAGS_DISABLED;
+      yr_bitmask_set(context->strings_temp_disabled, string->idx);
       return ERROR_SUCCESS;
     }
     else if (result == CALLBACK_ABORT || result == CALLBACK_ERROR)

libyara/scanner.c

@@ -188,6 +188,11 @@ static void _yr_scanner_clean_matches(YR_SCANNER* scanner)
       0,
       sizeof(YR_BITMASK) * YR_BITMASK_SIZE(scanner->rules->num_namespaces));
 
+  memset(
+      scanner->strings_temp_disabled,
+      0,
+      sizeof(YR_BITMASK) * YR_BITMASK_SIZE(scanner->rules->num_strings));
+
   memset(scanner->matches, 0, sizeof(YR_MATCHES) * scanner->rules->num_strings);
 
   memset(
@@ -227,6 +232,9 @@ YR_API int yr_scanner_create(YR_RULES* rules, YR_SCANNER** scanner)
   new_scanner->ns_unsatisfied_flags = (YR_BITMASK*) yr_calloc(
       sizeof(YR_BITMASK), YR_BITMASK_SIZE(rules->num_namespaces));
 
+  new_scanner->strings_temp_disabled = (YR_BITMASK*) yr_calloc(
+      sizeof(YR_BITMASK), YR_BITMASK_SIZE(rules->num_strings));
+
   new_scanner->matches = (YR_MATCHES*) yr_calloc(
       rules->num_strings, sizeof(YR_MATCHES));
 
@@ -310,6 +318,7 @@ YR_API void yr_scanner_destroy(YR_SCANNER* scanner)
 
   yr_free(scanner->rule_matches_flags);
   yr_free(scanner->ns_unsatisfied_flags);
+  yr_free(scanner->strings_temp_disabled);
   yr_free(scanner->matches);
   yr_free(scanner->unconfirmed_matches);
   yr_free(scanner);

tests/test-api.c

@@ -842,6 +842,24 @@ void test_runtime_warnings() {
   assert_true_expr(counters.rules_matching == 0);
   assert_true_expr(counters.rules_not_matching == 1);
 
+  // Repeat the same scan to ensure that the string that had a warning is
+  // enabled after the first scan. But first we must reset the counters.
+  counters.rules_not_matching = 0;
+  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) {
+    yr_rules_destroy(rules);
+    perror("yr_rules_scan_file");
+    exit(EXIT_FAILURE);
+  }
+
+  // The assertions here should be EXACTLY the same as the assertions above. We
+  // are making sure the string is disabled only for a single scan.
+  assert_true_expr(counters.rules_warning == 1);
+  assert_true_expr(counters.rules_matching == 0);
+  assert_true_expr(counters.rules_not_matching == 1);
+
   yr_rules_destroy(rules);
   yr_finalize();
 }