Skip to content

Commit f1d73d3

Browse files
Support distinct handling and configuration for DCHECK failures
Separates `DCHECK` failures from standard `CHECK` failures to enable granular severity assessment and issue tracking policies. In Chromium, `DCHECK` failures often carry different security and priority implications than production `CHECK` failures. While they may not always be treated as immediate security vulnerabilities, they present information disclosure risks if filed publicly. Current logic groups them together, preventing distinct visibility rules. Detailed changes: - **Stack Parsing:** Updates `stacktraces` regex constants to explicitly distinguish "DCHECK failed" from "Check failed/NOTREACHED", assigning the distinct crash type `DCHECK failure`. - **Security Implications:** Introduces the `DCHECKS_HAVE_SECURITY_IMPLICATION` environment variable to control whether DCHECKs are flagged as security issues per-fuzzer. - **Policy Engine:** Refactors `IssueTrackerPolicy` to support recursive configuration application. This allows nested conditions (e.g., `all` -> `non_security` -> `dcheck`) to apply specific labels, access limits, or priority levels based on the intersection of crash traits. This decouple the configuration depth from the code, enabling arbitrary nesting or rules and simplifying the addition of future condition types. Bug: https://issues.chromium.org/issues/406667202
1 parent 3612e16 commit f1d73d3

File tree

7 files changed

+251
-84
lines changed

7 files changed

+251
-84
lines changed

src/clusterfuzz/_internal/crash_analysis/crash_analyzer.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -320,16 +320,21 @@ def is_security_issue(crash_stacktrace, crash_type, crash_address):
320320
return True
321321

322322
if crash_type == 'CHECK failure':
323-
# TODO(ochang): Remove this once we pick up newer builds that distinguish
324-
# DCHECKs from CHECKs.
325323
checks_have_security_implication = environment.get_value(
326324
'CHECKS_HAVE_SECURITY_IMPLICATION', False)
327325
return checks_have_security_implication
328326

329-
# Debug CHECK failure should be marked with security implications.
330-
if crash_type in ('Security DCHECK failure', 'DCHECK failure'):
327+
# Security DCHECKs are always security issues.
328+
if crash_type == 'Security DCHECK failure':
331329
return True
332330

331+
# For regular DCHECKs, projects can choose whether or not a particular fuzzer
332+
# job treats them as security issues.
333+
if crash_type == 'DCHECK failure':
334+
dchecks_have_security_implication = environment.get_value(
335+
'DCHECKS_HAVE_SECURITY_IMPLICATION', True)
336+
return dchecks_have_security_implication
337+
333338
# Hard crash, explicitly enforced in code.
334339
if (crash_type == 'Fatal error' or crash_type == 'Unreachable code' or
335340
crash_type.endswith('Exception') or crash_type.endswith('CHECK failure')):

src/clusterfuzz/_internal/issue_management/issue_filer.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,9 @@ def file_issue(testcase,
323323
is_crash = not utils.sub_string_exists_in(NON_CRASH_TYPES,
324324
testcase.crash_type)
325325
properties = policy.get_new_issue_properties(
326-
is_security=testcase.security_flag, is_crash=is_crash)
326+
is_security=testcase.security_flag,
327+
is_crash=is_crash,
328+
crash_type=testcase.crash_type)
327329

328330
issue = issue_tracker.new_issue()
329331
issue.title = data_handler.get_issue_summary(testcase)

src/clusterfuzz/_internal/issue_management/issue_tracker_policy.py

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -146,62 +146,80 @@ def fallback_policy_message(self):
146146
"""Get the fallback policy message, if it exists."""
147147
return self._data.get('fallback_policy_message')
148148

149-
def get_new_issue_properties(self, is_security, is_crash):
149+
def get_new_issue_properties(self, is_security, is_crash, crash_type):
150150
"""Get the properties to apply to a new issue."""
151151
policy = NewIssuePolicy()
152152

153-
if 'all' in self._data:
154-
self._apply_new_issue_properties(policy, self._data['all'], is_crash)
153+
properties = {
154+
'is_security': is_security,
155+
'is_crash': is_crash,
156+
'is_dcheck': crash_type == 'DCHECK failure',
157+
}
155158

159+
self._apply_new_issue_properties(policy, self._data.get('all'),
160+
**properties)
161+
162+
# Legacy support: support top-level 'security' and 'non_security' keys. New
163+
# configs should use the 'all' section and add 'security' and
164+
# 'non_security' subsections within that.
156165
if is_security:
157-
if 'security' in self._data:
158-
self._apply_new_issue_properties(policy, self._data['security'],
159-
is_crash)
166+
self._apply_new_issue_properties(policy, self._data.get('security'),
167+
**properties)
160168
else:
161-
if 'non_security' in self._data:
162-
self._apply_new_issue_properties(policy, self._data['non_security'],
163-
is_crash)
169+
self._apply_new_issue_properties(policy, self._data.get('non_security'),
170+
**properties)
164171

165172
return policy
166173

167-
def _apply_new_issue_properties(self, policy, issue_type, is_crash):
174+
def _apply_new_issue_properties(self, policy, data, **properties):
168175
"""Apply issue policies."""
169-
if not issue_type:
176+
if not data:
170177
return
171178

172-
if 'status' in issue_type:
173-
policy.status = self._data['status'][issue_type['status']]
179+
if 'status' in data:
180+
policy.status = self._data['status'][data['status']]
174181

175-
if 'ccs' in issue_type:
176-
policy.ccs.extend(issue_type['ccs'])
182+
if 'ccs' in data:
183+
policy.ccs.extend(data['ccs'])
177184

178-
labels = issue_type.get('labels')
185+
labels = data.get('labels')
179186
if labels:
180187
policy.labels.extend(_to_str_list(labels))
181188

182-
issue_body_footer = issue_type.get('issue_body_footer')
189+
issue_body_footer = data.get('issue_body_footer')
183190
if issue_body_footer:
184191
policy.issue_body_footer = issue_body_footer
185192

186-
if is_crash:
187-
crash_labels = issue_type.get('crash_labels')
188-
if crash_labels:
189-
policy.labels.extend(_to_str_list(crash_labels))
190-
else:
191-
non_crash_labels = issue_type.get('non_crash_labels')
192-
if non_crash_labels:
193-
policy.labels.extend(_to_str_list(non_crash_labels))
194-
195-
for k, v in self.get_extension_fields(issue_type).items():
193+
for k, v in self.get_extension_fields(data).items():
196194
policy.extension_fields[k] = v
197195

196+
# Handle conditions recursively:
197+
children_conditions = {
198+
'security': properties.get('is_security', None) is True,
199+
'non_security': properties.get('is_security', None) is False,
200+
'crash': properties.get('is_crash', None) is True,
201+
'non_crash': properties.get('is_crash', None) is False,
202+
'dcheck': properties.get('is_dcheck', None) is True,
203+
'non_dcheck': properties.get('is_dcheck', None) is False,
204+
}
205+
for child, condition in children_conditions.items():
206+
if condition:
207+
self._apply_new_issue_properties(policy, data.get(child), **properties)
208+
209+
# Legacy support: support "crash_label" and "non_crash_label" keys.
210+
# New configs should use the "crash" and "non_crash" sections instead and
211+
# a "labels" list within those sections.
212+
if properties.get('is_crash', None) is False and 'crash_label' in data:
213+
policy.labels.append(str(data['crash_label']))
214+
if properties.get('is_crash', None) is True and 'non_crash_label' in data:
215+
policy.labels.append(str(data['non_crash_label']))
216+
198217
def get_existing_issue_properties(self):
199218
"""Get the properties to apply to a new issue."""
200219
policy = NewIssuePolicy()
201-
202-
if 'existing' in self._data:
203-
self._apply_new_issue_properties(policy, self._data['existing'], False)
204-
220+
properties = {} # No recursive properties for existing issues.
221+
self._apply_new_issue_properties(policy, self._data['existing'],
222+
**properties)
205223
return policy
206224

207225

0 commit comments

Comments
 (0)