avdtp: bound message assembler to drop truncated PDUs (DoS prevention)#918
Merged
zxzxwu merged 2 commits intogoogle:mainfrom Apr 27, 2026
Merged
Conversation
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 google#912 (empty ATT PDU) and google#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.
zxzxwu
approved these changes
Apr 27, 2026
| assembler = avdtp.MessageAssembler(callback) | ||
| # Must not raise; nothing should be delivered to callback either. | ||
| assembler.on_pdu(pdu) | ||
| assert completed == [] |
Collaborator
There was a problem hiding this comment.
Suggested change
| assert completed == [] | |
| assert not completed |
Or use mock.
| # 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: |
Collaborator
There was a problem hiding this comment.
Suggested change
| if len(pdu) < 1: | |
| if not pdu: |
Per @zxzxwu review on google#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.
Contributor
Author
|
Both nits addressed in 07b5e33 — |
Contributor
Author
|
Cross-reference for the Google VRP triage team: this PR is tracked at https://issuetracker.google.com/issues/506803574 (reporter: sactransport2000@gmail.com). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Truncated AVDTP frames from a remote peer crash the L2CAP channel by raising
IndexErrorinMessageAssembler.on_pdu.MessageAssembler.on_pduunconditionally indexespdu[0],pdu[1], and (for START packets)pdu[2]. A peer sending a 0-, 1-, or 2-byte AVDTP frame triggersIndexError, which propagates up through the L2CAP read loop and tears down the channel.This is the same DoS class as:
Fix
Validate length before each access. Empty PDUs and packets shorter than the type-specific minimum are logged at warning level and dropped; the assembler stays alive so the L2CAP channel is not torn down.
Both helpers in this fix mirror the conservative warn-and-drop approach already used in
MessageAssemblerfor unexpected continuations.Test plan
test_message_assembler_truncated_pducovering 4 truncated inputs (empty, 1-byte SINGLE, 1-byte START, 2-byte START) — all four raiseIndexErrorpre-fix and pass cleanly post-fix.tests/avdtp_test.pysuite passes (43 tests).black bumble/ tests/clean.