From 8614881fb3592f9cc32619399a99232494333223 Mon Sep 17 00:00:00 2001 From: ibondarenko1 Date: Thu, 23 Apr 2026 00:53:06 -0700 Subject: [PATCH] sdp: bound DataElement parse recursion to prevent RecursionError DoS DataElement.from_bytes -> list_from_bytes -> (SEQUENCE/ALTERNATIVE constructor dispatches back to list_from_bytes) had no depth limit. A malicious SDP peer could send a PDU of a few kilobytes containing ~1000 nested SEQUENCE tags and exhaust the Python recursion stack, crashing the host with an unhandled RecursionError propagating out of the SDP handler. Reachable via: any remote Bluetooth device that Bumble performs SDP service discovery against (default during Classic connection setup). Same family as PR #912 (ATT_PDU.from_bytes empty PDU IndexError) - remote unchecked-input parser crash in the Bluetooth stack. Fix: thread-local depth counter, cap nesting at 32 (well above anything a legitimate service record uses). Added two regression tests covering the deep-nesting reject path and normal 16-level-nested SEQUENCE parsing. Reproducer (4.5 KB payload, deterministic crash on 0.0.228): from bumble.sdp import DataElement inner = b"\x35\x00" for _ in range(1500): size = len(inner) if size < 65535: inner = bytes([0x36, (size >> 8) & 0xFF, size & 0xFF]) + inner DataElement.from_bytes(inner) # RecursionError before fix Signed-off-by: ibondarenko1 --- bumble/sdp.py | 30 ++++++++++++++++++++++++------ tests/sdp_test.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/bumble/sdp.py b/bumble/sdp.py index 969c867..07571a2 100644 --- a/bumble/sdp.py +++ b/bumble/sdp.py @@ -20,6 +20,7 @@ from __future__ import annotations import asyncio import logging import struct +import threading from collections.abc import Iterable, Sequence from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, ClassVar, NewType, TypeVar @@ -44,6 +45,13 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +# SDP data elements are nested (SEQUENCE, ALTERNATIVE). Cap parse recursion to +# prevent a malicious peer from crashing the process via a deeply nested PDU. +# 32 levels is well beyond anything a legitimate service record uses. +_MAX_DATA_ELEMENT_NESTING = 32 +_parse_state = threading.local() + + # ----------------------------------------------------------------------------- # Constants # ----------------------------------------------------------------------------- @@ -284,12 +292,22 @@ class DataElement: @staticmethod def list_from_bytes(data): - elements = [] - while data: - element = DataElement.from_bytes(data) - elements.append(element) - data = data[len(bytes(element)) :] - return elements + depth = getattr(_parse_state, "depth", 0) + if depth >= _MAX_DATA_ELEMENT_NESTING: + raise ValueError( + f"SDP data element nesting exceeds max depth " + f"({_MAX_DATA_ELEMENT_NESTING})" + ) + _parse_state.depth = depth + 1 + try: + elements = [] + while data: + element = DataElement.from_bytes(data) + elements.append(element) + data = data[len(bytes(element)) :] + return elements + finally: + _parse_state.depth = depth @staticmethod def parse_from_bytes(data, offset): diff --git a/tests/sdp_test.py b/tests/sdp_test.py index 31c569a..0acbf4b 100644 --- a/tests/sdp_test.py +++ b/tests/sdp_test.py @@ -440,3 +440,44 @@ async def run(): if __name__ == '__main__': logging.basicConfig(level=os.environ.get('BUMBLE_LOGLEVEL', 'INFO').upper()) asyncio.run(run()) + + +# ----------------------------------------------------------------------------- +def test_nested_sequence_recursion_guard(): + """Regression test: deeply-nested SDP SEQUENCE/ALTERNATIVE must not crash + the parser with RecursionError. Instead a ValueError is raised once the + configured nesting limit is exceeded. + + Root cause: DataElement.from_bytes -> list_from_bytes -> (constructor + dispatching back to list_from_bytes for SEQUENCE/ALTERNATIVE) recursed + without a depth limit. A malicious SDP peer could craft a PDU exceeding + Pythons default recursion limit (~1000 frames) and crash the host. + """ + # Build nested SEQUENCE payload with tag 0x36 (SEQUENCE, 2-byte length). + inner = b"\x35\x00" # empty SEQUENCE terminator + for _ in range(1500): + size = len(inner) + if size >= 65535: + break + inner = bytes([0x36, (size >> 8) & 0xFF, size & 0xFF]) + inner + + import pytest + with pytest.raises(ValueError, match="nesting exceeds max depth"): + DataElement.from_bytes(inner) + + +def test_nested_sequence_within_limit_still_works(): + """Nested-but-reasonable SDP SEQUENCEs must still parse correctly.""" + leaf = DataElement.unsigned_integer(1, value_size=2) + payload = leaf + for _ in range(16): # under the 32-depth limit + payload = DataElement.sequence([payload]) + raw = bytes(payload) + parsed = DataElement.from_bytes(raw) + # Walk back down to confirm structural integrity preserved + cur = parsed + for _ in range(16): + assert cur.type == DataElement.SEQUENCE + cur = cur.value[0] + assert cur.type == DataElement.UNSIGNED_INTEGER + assert cur.value == 1