Browse Source

feat(sdk-crashes): Keep stacktrace registers (#69489)

Stacktrace registers are useful for native and don't contain PII.
Therefore, we can keep them.

Fixes GH-68772
Philipp Hofmann 10 months ago
parent
commit
3939b518f2

+ 14 - 3
fixtures/sdk_crash_detection/crash_event_cocoa.py

@@ -102,12 +102,22 @@ def get_frames(
     return frames[::-1]
 
 
-def get_crash_event(handled=False, function="-[Sentry]", **kwargs) -> dict[str, Collection[str]]:
-    return get_crash_event_with_frames(get_frames(function), handled=handled, **kwargs)
+def get_crash_event(
+    handled=False,
+    function="-[Sentry]",
+    registers: Sequence[Mapping[str, str]] | None = None,
+    **kwargs,
+) -> dict[str, Collection[str]]:
+    return get_crash_event_with_frames(
+        get_frames(function), registers=registers, handled=handled, **kwargs
+    )
 
 
 def get_crash_event_with_frames(
-    frames: Sequence[Mapping[str, Any]], handled=False, **kwargs
+    frames: Sequence[Mapping[str, Any]],
+    registers: Sequence[Mapping[str, str]] | None = None,
+    handled=False,
+    **kwargs,
 ) -> dict[str, Collection[str]]:
     result = {
         "event_id": "80e3496eff734ab0ac993167aaa0d1cd",
@@ -121,6 +131,7 @@ def get_crash_event_with_frames(
                 {
                     "stacktrace": {
                         "frames": frames,
+                        "registers": registers,
                     },
                     "type": "EXC_BAD_ACCESS",
                     "value": "crash > crash: > objectAtIndex: >\nAttempted to dereference null pointer.",

+ 17 - 1
src/sentry/utils/sdk_crashes/event_stripper.py

@@ -14,6 +14,9 @@ class Allow(Enum):
     """Keeps the event data if it is of type str, int, float, bool."""
     SIMPLE_TYPE = auto()
 
+    """Keeps the event data if it is a mapping with string keys and string values."""
+    MAP_WITH_STRINGS = auto()
+
     """
     Doesn't keep the event data no matter the type. This can be used to explicitly
     specify that data should be removed with an explanation.
@@ -55,7 +58,10 @@ EVENT_DATA_ALLOWLIST = {
                     "package": Allow.SIMPLE_TYPE,
                     "platform": Allow.SIMPLE_TYPE,
                     "lineno": Allow.SIMPLE_TYPE,
-                }
+                },
+                "registers": Allow.MAP_WITH_STRINGS.with_explanation(
+                    "Registers contain memory addresses, which isn't PII."
+                ),
             },
             "value": Allow.NEVER.with_explanation("The exception value could contain PII."),
             "type": Allow.SIMPLE_TYPE,
@@ -147,6 +153,16 @@ def _strip_event_data_with_allowlist(
 
             if allowed is Allow.SIMPLE_TYPE and isinstance(data_value, (str, int, float, bool)):
                 stripped_data[data_key] = data_value
+            elif allowed is Allow.MAP_WITH_STRINGS and isinstance(data_value, Mapping):
+                map_with_hex_values = {}
+                for key, value in data_value.items():
+                    if isinstance(key, str) and isinstance(value, str):
+                        map_with_hex_values[key] = value
+
+                if len(map_with_hex_values) > 0:
+                    stripped_data[data_key] = map_with_hex_values
+
+                continue
             else:
                 continue
 

+ 40 - 0
tests/sentry/utils/sdk_crashes/test_event_stripper.py

@@ -394,6 +394,46 @@ def test_strip_frames_with_keep_for_fields_path_replacer(store_and_strip_event,
     }
 
 
+@pytest.mark.parametrize(
+    [
+        "registers",
+        "expected_registers",
+    ],
+    [
+        (None, None),
+        ({"x1": "10", "x2": "0x0"}, {"x1": "0xa", "x2": "0x0"}),
+        (
+            {
+                "fp": "0x16f8f6e90",
+                "lr": "0x10050ad74",
+                "pc": "0x10050ad8c",
+                "sp": "0x16f8f6e30",
+                "x0": "0x0",
+                "x10": "0x2",
+                "x12": "0x10000000000",
+            },
+            {
+                "fp": "0x16f8f6e90",
+                "lr": "0x10050ad74",
+                "pc": "0x10050ad8c",
+                "sp": "0x16f8f6e30",
+                "x0": "0x0",
+                "x10": "0x2",
+                "x12": "0x10000000000",
+            },
+        ),
+    ],
+)
+@django_db_all
+def test_event_data_with_registers(registers, expected_registers, store_and_strip_event):
+    stripped_event_data = store_and_strip_event(data=get_crash_event(registers=registers))
+
+    stripped_registers = get_path(
+        stripped_event_data, "exception", "values", -1, "stacktrace", "registers"
+    )
+    assert stripped_registers == expected_registers
+
+
 @django_db_all
 @pytest.mark.snuba
 def test_strip_event_without_data_returns_empty_dict(store_and_strip_event):