From b874e26a4f644fc876856dbdabc181e2fbf3dc11 Mon Sep 17 00:00:00 2001 From: ibondarenko1 Date: Sun, 26 Apr 2026 18:16:15 -0700 Subject: [PATCH 1/2] avdtp: bound message assembler to drop truncated PDUs (DoS prevention) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A remote peer can send an AVDTP frame shorter than the assembler expects. The current MessageAssembler.on_pdu() unconditionally accesses pdu[0], pdu[1], and (for START packets) pdu[2], so a 0-, 1-, or 2-byte frame raises IndexError. The exception propagates up through L2CAP's read loop and tears down the channel — same DoS class as #912 (empty ATT PDU) and #914 (unbounded SDP recursion). Fix: validate length before each access. Empty PDUs and packets shorter than the type-specific minimum are logged and dropped; the assembler stays alive so the L2CAP channel is not torn down. - bumble/avdtp.py: length guards in MessageAssembler.on_pdu before accessing pdu[0], pdu[1], pdu[2]. - tests/avdtp_test.py: regression test covering empty PDU, 1-byte SINGLE, 1-byte START, 2-byte START — all four would have raised IndexError pre-fix; assembler now drops without raising. --- bumble/avdtp.py | 24 ++++++++++++++++++++++++ tests/avdtp_test.py | 25 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/bumble/avdtp.py b/bumble/avdtp.py index e968ec1..becafbd 100644 --- a/bumble/avdtp.py +++ b/bumble/avdtp.py @@ -311,6 +311,13 @@ class MessageAssembler: def on_pdu(self, pdu: bytes) -> None: self.packet_count += 1 + # Drop empty PDUs sent by remote — accessing pdu[0] below would + # raise IndexError, propagating up to the L2CAP read loop and + # tearing down the channel. Same class as #912 (ATT empty PDU). + if len(pdu) < 1: + logger.warning('AVDTP message assembler: empty PDU dropped') + return + transaction_label = pdu[0] >> 4 packet_type = Protocol.PacketType((pdu[0] >> 2) & 3) message_type = Message.MessageType(pdu[0] & 3) @@ -324,6 +331,23 @@ class MessageAssembler: Protocol.PacketType.SINGLE_PACKET, Protocol.PacketType.START_PACKET, ): + # Both single and start packets carry the signal identifier in + # pdu[1]; start packets additionally carry the packet count in + # pdu[2]. Guard each access so a malformed remote frame can't + # crash the message assembler. + if len(pdu) < 2: + logger.warning( + 'AVDTP %s packet too short (%d bytes); dropped', + packet_type.name, + len(pdu), + ) + return + if packet_type == Protocol.PacketType.START_PACKET and len(pdu) < 3: + logger.warning( + 'AVDTP START packet missing signal-packet count; dropped' + ) + return + if self.message is not None: # The previous message has not been terminated logger.warning( diff --git a/tests/avdtp_test.py b/tests/avdtp_test.py index f2fbd12..d5df431 100644 --- a/tests/avdtp_test.py +++ b/tests/avdtp_test.py @@ -120,6 +120,31 @@ def test_messages(message: avdtp.Message): assert message.payload == parsed.payload +# ----------------------------------------------------------------------------- +@pytest.mark.parametrize( + 'pdu', + ( + b'', # empty PDU — would IndexError on pdu[0] + b'\x00', # 1-byte SINGLE_PACKET — would IndexError on pdu[1] + b'\x04', # 1-byte START_PACKET — would IndexError on pdu[1] + b'\x44\x10', # 2-byte START_PACKET — would IndexError on pdu[2] + ), +) +def test_message_assembler_truncated_pdu(pdu: bytes): + """Truncated AVDTP PDUs from a remote peer must NOT raise IndexError — + same DoS class as #912 (ATT empty PDU). The assembler is required to + log + drop and stay alive so the L2CAP channel survives.""" + completed = [] + + def callback(transaction_label, message): + completed.append((transaction_label, message)) + + assembler = avdtp.MessageAssembler(callback) + # Must not raise; nothing should be delivered to callback either. + assembler.on_pdu(pdu) + assert completed == [] + + # ----------------------------------------------------------------------------- def test_rtp(): packet = bytes.fromhex( From 07b5e33e09e6ea16e42bd73ed10e0f3da9b0e5c4 Mon Sep 17 00:00:00 2001 From: ibondarenko1 Date: Sun, 26 Apr 2026 18:49:55 -0700 Subject: [PATCH 2/2] =?UTF-8?q?avdtp:=20address=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20use=20truthy=20checks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @zxzxwu review on #918: - bumble/avdtp.py: replace `if len(pdu) < 1:` with `if not pdu:` - tests/avdtp_test.py: replace `assert completed == []` with `assert not completed` Both are idiomatic Python truthy checks; behavior identical. --- bumble/avdtp.py | 2 +- tests/avdtp_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bumble/avdtp.py b/bumble/avdtp.py index becafbd..9e9b836 100644 --- a/bumble/avdtp.py +++ b/bumble/avdtp.py @@ -314,7 +314,7 @@ class MessageAssembler: # Drop empty PDUs sent by remote — accessing pdu[0] below would # raise IndexError, propagating up to the L2CAP read loop and # tearing down the channel. Same class as #912 (ATT empty PDU). - if len(pdu) < 1: + if not pdu: logger.warning('AVDTP message assembler: empty PDU dropped') return diff --git a/tests/avdtp_test.py b/tests/avdtp_test.py index d5df431..242bdae 100644 --- a/tests/avdtp_test.py +++ b/tests/avdtp_test.py @@ -142,7 +142,7 @@ def test_message_assembler_truncated_pdu(pdu: bytes): assembler = avdtp.MessageAssembler(callback) # Must not raise; nothing should be delivered to callback either. assembler.on_pdu(pdu) - assert completed == [] + assert not completed # -----------------------------------------------------------------------------