Prevent undefined values in offset ranges from corrupting the VM's st…

…ack.
VirusTotal · Aug 8, 2022 · b77e4f4 · b77e4f4
1 parent f90cd22
commit b77e4f4
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 24 deletions.
diff --git a/libyara/exec.c b/libyara/exec.c
@@ -1610,11 +1610,20 @@ int yr_execute_code(YR_SCAN_CONTEXT* context)
 
       pop(r2);  // Offset range end
       pop(r1);  // Offset range start
+      pop(r3);  // First string
 
-      ensure_defined(r1);
-      ensure_defined(r2);
-
-      pop(r3);
+      // If any of the range boundaries are undefined the result is also
+      // undefined, be we need to unwind the stack first.
+      if (is_undef(r1) || is_undef(r2))
+      {
+        // Remove all the strings.
+        while (!is_undef(r3)) pop(r3);
+        // Remove the quantifier at the bottom of the stack.
+        pop(r3);
+        r1.i = YR_UNDEFINED;
+        push(r1);
+        break;
+      }
 
       while (!is_undef(r3))
       {

diff --git a/tests/test-rules.c b/tests/test-rules.c
@@ -1947,6 +1947,38 @@ static void test_of()
        }",
       "mississippi");
 
+  // If one of the bounds can not be determined statically it isn't an error.
+  assert_true_rule(
+      "rule test { \
+      strings: \
+        $a = \"AXSERS\" \
+      condition: \
+        true or any of them in (0..filesize-100) \
+    }",
+      TEXT_1024_BYTES);
+
+  // Lower bound can not be negative, if it can be determined statically.
+  assert_error(
+      "rule test { \
+        strings: \
+          $a = \"AXSERS\" \
+        condition: \
+          $a in (-1..10) \
+      }",
+      ERROR_INVALID_VALUE);
+
+  // Make sure that an undefined range boundary returns an undefined value,
+  // which translates to false.
+  assert_false_rule(
+      "import \"tests\" \
+        rule test { \
+		      strings: \
+			      $a = \"missi\" \
+		      condition: \
+			      any of them in (0..tests.undefined.i) \
+	    }",
+      "mississippi");
+
   YR_DEBUG_FPRINTF(1, stderr, "} // %s()\n", __FUNCTION__);
 }
 
@@ -2184,26 +2216,6 @@ void test_for()
       }",
       ERROR_INVALID_VALUE);
 
-  // If one of the bounds can not be determined statically it isn't an error.
-  assert_true_rule(
-      "rule test { \
-      strings: \
-        $a = \"AXSERS\" \
-      condition: \
-        true or any of them in (0..filesize-100) \
-    }",
-      TEXT_1024_BYTES);
-
-  // Lower bound can not be negative, if it can be determined statically.
-  assert_error(
-      "rule test { \
-        strings: \
-          $a = \"AXSERS\" \
-        condition: \
-          $a in (-1..10) \
-      }",
-      ERROR_INVALID_VALUE);
-
   // Test case for https://github.com/VirusTotal/yara/issues/1729
   assert_true_rule(
       "rule test { \