MiraclePtr-protected crashes are not security issues. (#4906)

As of M128, if a bug is marked MiraclePtr Status:PROTECTED, it is not
considered a security issue. It should be converted to type:Bug and
assigned to the appropriate engineering team as functional issue.

This patch moves the MiraclePtr detection logic from `issue_filer.py` to
`crash_analyzer.py` and uses it to correctly classify
MiraclePtr-protected crashes as non-security issues.

Fixed:#4903
Fixed:https://crbug.com/40930527

Author: ArthurSonzogni

src/clusterfuzz/_internal/crash_analysis/crash_analyzer.py

@@ -16,6 +16,7 @@
 import re
 
 from clusterfuzz._internal.config import local_config
+from clusterfuzz._internal.metrics import logs
 from clusterfuzz._internal.system import environment
 
 ASSERT_CRASH_ADDRESSES = [
@@ -133,6 +134,14 @@
 # Default page size of 4KB.
 NULL_DEREFERENCE_BOUNDARY = 0x1000
 
+CHROMIUM_MIRACLEPTR_REGEX = re.compile(r'.*MiraclePtr Status:.+')
+
+MIRACLEPTR_STATUS = {
+    'PROTECTED': 'MiraclePtr-Protected',
+    'MANUAL ANALYSIS REQUIRED': 'MiraclePtr-ManualAnalysisRequired',
+    'NOT PROTECTED': 'MiraclePtr-NotProtected'
+}
+
 
 def address_to_integer(address):
   """Attempt to convert an address from a string (hex) to an integer."""
@@ -268,9 +277,26 @@ def has_signal_for_non_security_bug_type(stacktrace):
   return False
 
 
+def check_miracleptr_status(stacktrace):
+  """Look for MiraclePtr status string and return the appropriate label."""
+  for line in stacktrace.split('\n'):
+    if CHROMIUM_MIRACLEPTR_REGEX.match(line):
+      status = line.split(':')[-1].strip()
+      try:
+        return MIRACLEPTR_STATUS[status]
+      except KeyError:
+        logs.error(f'Unknown MiraclePtr status: {line}')
+        break
+  return None
+
+
 def is_security_issue(crash_stacktrace, crash_type, crash_address):
   """Based on unsymbolized crash parameters, determine whether it has security
   consequences or not."""
+  # MiraclePtr protected crashes are not security issues.
+  if check_miracleptr_status(crash_stacktrace) == 'MiraclePtr-Protected':
+    return False
+
   # Stack traces of any type can be manually labelled as a security issue.
   if re.search('FuzzerSecurityIssue(Critical|High|Medium|Low)',
                crash_stacktrace):

src/clusterfuzz/_internal/issue_management/issue_filer.py

@@ -19,6 +19,7 @@
 from clusterfuzz._internal.base import external_users
 from clusterfuzz._internal.base import utils
 from clusterfuzz._internal.config import local_config
+from clusterfuzz._internal.crash_analysis import crash_analyzer
 from clusterfuzz._internal.crash_analysis import severity_analyzer
 from clusterfuzz._internal.datastore import data_handler
 from clusterfuzz._internal.datastore import data_types
@@ -74,13 +75,6 @@
 ]
 
 STACKFRAME_LINE_REGEX = re.compile(r'\s*#\d+\s+0x[0-9A-Fa-f]+\s*')
-CHROMIUM_MIRACLEPTR_REGEX = re.compile(r'.*MiraclePtr Status:.+')
-
-MIRACLEPTR_STATUS = {
-    'PROTECTED': 'MiraclePtr-Protected',
-    'MANUAL ANALYSIS REQUIRED': 'MiraclePtr-ManualAnalysisRequired',
-    'NOT PROTECTED': 'MiraclePtr-NotProtected'
-}
 
 
 def platform_substitution(label, testcase, _):
@@ -317,20 +311,6 @@ def notify_issue_update(testcase, status):
     oss_fuzz_github.close_issue(testcase)
 
 
-def check_miracleptr_status(testcase):
-  """Look for MiraclePtr status string and return the appropriate label."""
-  stacktrace = data_handler.get_stacktrace(testcase)
-  for line in stacktrace.split('\n'):
-    if CHROMIUM_MIRACLEPTR_REGEX.match(line):
-      status = line.split(':')[-1].strip()
-      try:
-        return MIRACLEPTR_STATUS[status]
-      except:
-        logs.error(f'Unknown MiraclePtr status: {line}')
-        break
-  return None
-
-
 def file_issue(testcase,
                issue_tracker,
                security_severity=None,
@@ -370,7 +350,8 @@ def file_issue(testcase,
       update_issue_impact_labels(testcase, issue, policy)
 
     # Check for MiraclePtr in stacktrace.
-    miracle_label = check_miracleptr_status(testcase)
+    stacktrace = data_handler.get_stacktrace(testcase)
+    miracle_label = crash_analyzer.check_miracleptr_status(stacktrace)
     if miracle_label:
       issue.labels.add(policy.substitution_mapping(miracle_label))
 

src/clusterfuzz/_internal/tests/core/crash_analysis/stack_parsing/stack_analyzer_data/miracle_ptr_manual_analysis_required.txt

@@ -0,0 +1,16 @@
+==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f00003b280 at pc 0x7efd4a2a3e03 bp 0x7ffd1ed50680 sp 0x7ffd1ed50678
+READ of size 8 at 0x60f00003b280 thread T0
+    #0 0x7efd4a2a3e02 in function_a() src/a.cc:1:1
+    #1 0x7efd4a29f60e in function_b() src/b.cc:1:1
+
+MiraclePtr Status: MANUAL ANALYSIS REQUIRED
+
+0x60f00003b280 is located 16 bytes inside of 112-byte region [0x60f00003b270,0x60f00003b2e0)
+freed by thread T0 here:
+    #0 0x7efd42e32fdb in __interceptor_free
+    #1 0x7efd4a19be58 in function_c() src/c.cc:1:1
+previously allocated by thread T0 here:
+    #0 0x7efd42e332fb in __interceptor_malloc
+    #1 0x7efd476d64aa in function_d() src/d.cc:1:1
+
+SUMMARY: AddressSanitizer: heap-use-after-free src/a.cc:1:1 in function_a()

src/clusterfuzz/_internal/tests/core/crash_analysis/stack_parsing/stack_analyzer_data/miracle_ptr_not_protected.txt

@@ -0,0 +1,16 @@
+==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f00003b280 at pc 0x7efd4a2a3e03 bp 0x7ffd1ed50680 sp 0x7ffd1ed50678
+READ of size 8 at 0x60f00003b280 thread T0
+    #0 0x7efd4a2a3e02 in function_a() src/a.cc:1:1
+    #1 0x7efd4a29f60e in function_b() src/b.cc:1:1
+
+MiraclePtr Status: NOT PROTECTED
+
+0x60f00003b280 is located 16 bytes inside of 112-byte region [0x60f00003b270,0x60f00003b2e0)
+freed by thread T0 here:
+    #0 0x7efd42e32fdb in __interceptor_free
+    #1 0x7efd4a19be58 in function_c() src/c.cc:1:1
+previously allocated by thread T0 here:
+    #0 0x7efd42e332fb in __interceptor_malloc
+    #1 0x7efd476d64aa in function_d() src/d.cc:1:1
+
+SUMMARY: AddressSanitizer: heap-use-after-free src/a.cc:1:1 in function_a()

src/clusterfuzz/_internal/tests/core/crash_analysis/stack_parsing/stack_analyzer_data/miracle_ptr_protected.txt

@@ -0,0 +1,16 @@
+==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f00003b280 at pc 0x7efd4a2a3e03 bp 0x7ffd1ed50680 sp 0x7ffd1ed50678
+READ of size 8 at 0x60f00003b280 thread T0
+    #0 0x7efd4a2a3e02 in function_a() src/a.cc:1:1
+    #1 0x7efd4a29f60e in function_b() src/b.cc:1:1
+
+MiraclePtr Status: PROTECTED
+
+0x60f00003b280 is located 16 bytes inside of 112-byte region [0x60f00003b270,0x60f00003b2e0)
+freed by thread T0 here:
+    #0 0x7efd42e32fdb in __interceptor_free
+    #1 0x7efd4a19be58 in function_c() src/c.cc:1:1
+previously allocated by thread T0 here:
+    #0 0x7efd42e332fb in __interceptor_malloc
+    #1 0x7efd476d64aa in function_d() src/d.cc:1:1
+
+SUMMARY: AddressSanitizer: heap-use-after-free src/a.cc:1:1 in function_a()

src/clusterfuzz/_internal/tests/core/crash_analysis/stack_parsing/stack_analyzer_data/miracle_ptr_status_missing.txt

@@ -0,0 +1,14 @@
+==1==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f00003b280 at pc 0x7efd4a2a3e03 bp 0x7ffd1ed50680 sp 0x7ffd1ed50678
+READ of size 8 at 0x60f00003b280 thread T0
+    #0 0x7efd4a2a3e02 in function_a() src/a.cc:1:1
+    #1 0x7efd4a29f60e in function_b() src/b.cc:1:1
+
+0x60f00003b280 is located 16 bytes inside of 112-byte region [0x60f00003b270,0x60f00003b2e0)
+freed by thread T0 here:
+    #0 0x7efd42e32fdb in __interceptor_free
+    #1 0x7efd4a19be58 in function_c() src/c.cc:1:1
+previously allocated by thread T0 here:
+    #0 0x7efd42e332fb in __interceptor_malloc
+    #1 0x7efd476d64aa in function_d() src/d.cc:1:1
+
+SUMMARY: AddressSanitizer: heap-use-after-free src/a.cc:1:1 in function_a()

src/clusterfuzz/_internal/tests/core/crash_analysis/stack_parsing/stack_analyzer_test.py

@@ -3890,3 +3890,75 @@ def test_missing_libfuzzer_stacktrace(self):
     self._validate_get_crash_data(data, expected_type, expected_address,
                                   expected_state, expected_stacktrace,
                                   expected_security_flag, expected_categories)
+
+  def test_miracle_ptr_protected(self):
+    """Test for a MiraclePtr protected crash."""
+    data = self._read_test_data("miracle_ptr_protected.txt")
+    expected_type = "Heap-use-after-free\nREAD 8"
+    expected_address = "0x60f00003b280"
+    expected_state = "function_a\nfunction_b\nfunction_c\n"
+    expected_stacktrace = data
+    expected_security_flag = False
+
+    self._validate_get_crash_data(
+        data,
+        expected_type,
+        expected_address,
+        expected_state,
+        expected_stacktrace,
+        expected_security_flag,
+    )
+
+  def test_miracle_ptr_not_protected(self):
+    """Test for a MiraclePtr not protected crash."""
+    data = self._read_test_data("miracle_ptr_not_protected.txt")
+    expected_type = "Heap-use-after-free\nREAD 8"
+    expected_address = "0x60f00003b280"
+    expected_state = "function_a\nfunction_b\nfunction_c\n"
+    expected_stacktrace = data
+    expected_security_flag = True
+
+    self._validate_get_crash_data(
+        data,
+        expected_type,
+        expected_address,
+        expected_state,
+        expected_stacktrace,
+        expected_security_flag,
+    )
+
+  def test_miracle_ptr_manual_analysis_required(self):
+    """Test for a MiraclePtr crash that requires manual analysis."""
+    data = self._read_test_data("miracle_ptr_manual_analysis_required.txt")
+    expected_type = "Heap-use-after-free\nREAD 8"
+    expected_address = "0x60f00003b280"
+    expected_state = "function_a\nfunction_b\nfunction_c\n"
+    expected_stacktrace = data
+    expected_security_flag = True
+
+    self._validate_get_crash_data(
+        data,
+        expected_type,
+        expected_address,
+        expected_state,
+        expected_stacktrace,
+        expected_security_flag,
+    )
+
+  def test_miracle_ptr_status_missing(self):
+    """Test for a crash where MiraclePtr status is missing."""
+    data = self._read_test_data("miracle_ptr_status_missing.txt")
+    expected_type = "Heap-use-after-free\nREAD 8"
+    expected_address = "0x60f00003b280"
+    expected_state = "function_a\nfunction_b\nfunction_c\n"
+    expected_stacktrace = data
+    expected_security_flag = True
+
+    self._validate_get_crash_data(
+        data,
+        expected_type,
+        expected_address,
+        expected_state,
+        expected_stacktrace,
+        expected_security_flag,
+    )