From 9a7022e2382818df524d29bafd745bd62c096604 Mon Sep 17 00:00:00 2001 From: Will Kahn-Greene Date: Wed, 28 Feb 2024 09:26:30 -0500 Subject: [PATCH] bug-1830954: add support for has_guard_page_access 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. --- socorro/external/es/super_search_fields.py | 1 + socorro/mozilla_rulesets.py | 2 + socorro/processor/rules/breakpad.py | 28 ++++++++ socorro/schemas/processed_crash.schema.yaml | 14 ++++ .../tests/processor/rules/test_breakpad.py | 65 +++++++++++++++++++ .../jinja2/crashstats/report_index.html | 9 +++ 6 files changed, 119 insertions(+) diff --git a/socorro/external/es/super_search_fields.py b/socorro/external/es/super_search_fields.py index b0634a6cd6..5b04b7d6c8 100644 --- a/socorro/external/es/super_search_fields.py +++ b/socorro/external/es/super_search_fields.py @@ -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", diff --git a/socorro/mozilla_rulesets.py b/socorro/mozilla_rulesets.py index fd927b1ba4..4628e4b73b 100644 --- a/socorro/mozilla_rulesets.py +++ b/socorro/mozilla_rulesets.py @@ -10,6 +10,7 @@ ) from socorro.processor.rules.breakpad import ( CrashingThreadInfoRule, + HasGuardPageAccessRule, MinidumpSha256HashRule, MinidumpStackwalkRule, PossibleBitFlipsRule, @@ -87,6 +88,7 @@ CrashingThreadInfoRule(), TruncateStacksRule(), PossibleBitFlipsRule(), + HasGuardPageAccessRule(), MajorVersionRule(), PluginRule(), AccessibilityRule(), diff --git a/socorro/processor/rules/breakpad.py b/socorro/processor/rules/breakpad.py index 3ee4ea1b49..af5c588eca 100644 --- a/socorro/processor/rules/breakpad.py +++ b/socorro/processor/rules/breakpad.py @@ -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 diff --git a/socorro/schemas/processed_crash.schema.yaml b/socorro/schemas/processed_crash.schema.yaml index bc786adcdd..b397455e87 100644 --- a/socorro/schemas/processed_crash.schema.yaml +++ b/socorro/schemas/processed_crash.schema.yaml @@ -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"] @@ -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: > diff --git a/socorro/tests/processor/rules/test_breakpad.py b/socorro/tests/processor/rules/test_breakpad.py index 21ef692a51..4a8320f242 100644 --- a/socorro/tests/processor/rules/test_breakpad.py +++ b/socorro/tests/processor/rules/test_breakpad.py @@ -14,6 +14,7 @@ from socorro.processor.rules.breakpad import ( execute_process, CrashingThreadInfoRule, + HasGuardPageAccessRule, MinidumpSha256HashRule, MinidumpStackwalkRule, PossibleBitFlipsRule, @@ -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 = {} diff --git a/webapp/crashstats/crashstats/jinja2/crashstats/report_index.html b/webapp/crashstats/crashstats/jinja2/crashstats/report_index.html index 86cec83918..0b574fcff2 100644 --- a/webapp/crashstats/crashstats/jinja2/crashstats/report_index.html +++ b/webapp/crashstats/crashstats/jinja2/crashstats/report_index.html @@ -329,6 +329,15 @@

{{ report.product }} {{ report.version }} Crash Report [@ {{ report.signatur {% endif %} + {% if report.has_guard_page_access %} + + Has guard page access + + 
{{ report.has_guard_page_access }}
+ + + {% endif %} + {% if report.possible_bit_flips_max_confidence %} Possible bit flips max confidence