Skip to content

Commit

Permalink
bug-1830954: add support for has_guard_page_access
Browse files Browse the repository at this point in the history
This adds is_likely_guard_page to the stackwalker output part of the
processed crash schema.

It also adds a new top-level has_guard_page_access protected field. This
is true iff there's at least one memory access where
is_likely_guard_page is true.

These fields are marked protected because there are security concerns
with this data.
  • Loading branch information
willkg committed Feb 29, 2024
1 parent 6c357c7 commit 9a7022e
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 0 deletions.
1 change: 1 addition & 0 deletions socorro/external/es/super_search_fields.py
Expand Up @@ -1206,6 +1206,7 @@ def apply_schema_properties(fields, schema):
"query_type": "bool",
"storage_mapping": {"type": "boolean"},
},
"has_guard_page_access": boolean_field(name="has_guard_page_access"),
"has_mac_boot_args": boolean_field(name="has_mac_boot_args"),
"install_age": {
"data_validation_type": "int",
Expand Down
2 changes: 2 additions & 0 deletions socorro/mozilla_rulesets.py
Expand Up @@ -10,6 +10,7 @@
)
from socorro.processor.rules.breakpad import (
CrashingThreadInfoRule,
HasGuardPageAccessRule,
MinidumpSha256HashRule,
MinidumpStackwalkRule,
PossibleBitFlipsRule,
Expand Down Expand Up @@ -87,6 +88,7 @@
CrashingThreadInfoRule(),
TruncateStacksRule(),
PossibleBitFlipsRule(),
HasGuardPageAccessRule(),
MajorVersionRule(),
PluginRule(),
AccessibilityRule(),
Expand Down
28 changes: 28 additions & 0 deletions socorro/processor/rules/breakpad.py
Expand Up @@ -124,6 +124,34 @@ def action(self, raw_crash, dumps, processed_crash, tmpdir, status):
)


class HasGuardPageAccessRule(Rule):
"""Compute has_guard_page_access value
Fills in:
* has_guard_page_access (bool): whether there are "is_likely_guard_page=True"
in json_dump.crash_info.memory_accesses structures
"""

def action(self, raw_crash, dumps, processed_crash, tmpdir, status):
accesses = glom.glom(
processed_crash, "json_dump.crash_info.memory_accesses", default=None
)
if accesses is None:
return

# is_likely_guard_page is True iff at least one of the memory_accesses values is
# true
has_guard_page_access = any(
data.get("is_likely_guard_page", False) for data in accesses
)

# Only set the property if it's True
if has_guard_page_access:
processed_crash["has_guard_page_access"] = True


class TruncateStacksRule(Rule):
"""Truncate stacks that are too large
Expand Down
14 changes: 14 additions & 0 deletions socorro/schemas/processed_crash.schema.yaml
Expand Up @@ -199,6 +199,13 @@ definitions:
Size of memory access.
type: ["integer", "null"]
permissions: ["public"]
is_likely_guard_page:
description: >
Whether the address falls in a likely guard page (typically
indicating buffer overflow). This field may only be present
when the value is `true`.
type: ["boolean", "null"]
permissions: ["protected"]
type: ["object"]
permissions: ["public"]
permissions: ["public"]
Expand Down Expand Up @@ -2631,6 +2638,13 @@ properties:
type: boolean
permissions: ["public"]
source_annotation: IsGarbageCollecting
has_guard_page_access:
description: >
Set to `true` if and only if at least one of the memory accesses fell in
a likely guard page per the stackwalker output. Otherwise, this is
omitted.
type: ["boolean", "null"]
permissions: ["protected"]
java_exception:
$ref: "#/definitions/java_exception"
description: >
Expand Down
65 changes: 65 additions & 0 deletions socorro/tests/processor/rules/test_breakpad.py
Expand Up @@ -14,6 +14,7 @@
from socorro.processor.rules.breakpad import (
execute_process,
CrashingThreadInfoRule,
HasGuardPageAccessRule,
MinidumpSha256HashRule,
MinidumpStackwalkRule,
PossibleBitFlipsRule,
Expand Down Expand Up @@ -369,6 +370,70 @@ def test_address_scenarios(self, tmp_path, json_dump, expected):
assert processed_crash == expected


class TestHasGuardPageAccessRule:
def test_no_data(self, tmp_path):
raw_crash = {}
dumps = {}
processed_crash = {}
status = Status()

rule = HasGuardPageAccessRule()

rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status)

assert "has_guard_page_access" not in processed_crash

def test_no_memory_accesses(self, tmp_path):
raw_crash = {}
dumps = {}
processed_crash = {"json_dump": {"crash_info": {"memory_accesses": []}}}
status = Status()

rule = HasGuardPageAccessRule()

rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status)

assert "has_guard_page_access" not in processed_crash

def test_false(self, tmp_path):
raw_crash = {}
dumps = {}
processed_crash = {
"json_dump": {
"crash_info": {"memory_accesses": [{"is_likely_guard_page": False}, {}]}
}
}
status = Status()

rule = HasGuardPageAccessRule()

rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status)

assert "has_guard_page_access" not in processed_crash

def test_first_true(self, tmp_path):
raw_crash = {}
dumps = {}
processed_crash = {
"json_dump": {
"crash_info": {
"memory_accesses": [
{"is_likely_guard_page": False},
{},
{"is_likely_guard_page": True},
]
}
}
}
status = Status()

rule = HasGuardPageAccessRule()

rule.act(raw_crash, dumps, processed_crash, str(tmp_path), status)

assert processed_crash["has_guard_page_access"] is True


class TestPossibleBitFlipsRule:
def test_no_data(self, tmp_path):
raw_crash = {}
Expand Down
Expand Up @@ -329,6 +329,15 @@ <h2>{{ report.product }} {{ report.version }} Crash Report [@ {{ report.signatur
</tr>
{% endif %}

{% if report.has_guard_page_access %}
<tr title="{{ fields_desc['processed_crash.has_guard_page_access'] }}">
<th scope="row">Has guard page access</th>
<td>
<pre>{{ report.has_guard_page_access }}</pre>
</td>
</tr>
{% endif %}

{% if report.possible_bit_flips_max_confidence %}
<tr title="{{ fields_desc['processed_crash.possible_bit_flips_max_confidence'] }}">
<th scope="row">Possible bit flips max confidence</th>
Expand Down

0 comments on commit 9a7022e

Please sign in to comment.