Conversation
RTP packets may carry tail padding (RFC 3550 section 5.1). When the P bit is set in the header, the last octet of the payload gives the padding length, including itself. py-cord parses the P bit into RTPPacket.padding but never acts on it. This was harmless while the payload went straight to Opus, which ignores trailing garbage. It is not harmless with DAVE: the E2EE layer looks for its frame marker at the tail of the payload, so padding buries it and DaveSession.decrypt raises DecryptionFailed. In practice this shows up as choppy audio -- a large fraction of received frames silently replaced with OPUS_SILENCE -- rather than as an obvious error. discord.js hit the same bug and fixed it the same way in #11449. Fixes the receive-side decryption failures for padded packets.
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/3386/head:pr-3386
git checkout pr-3386This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/3386/head |
Contributor
|
This pull request does not follow the required pull request template. Please use the default template ( Problems detected: |
Member
|
idiot |
Member
|
Just FWIW, this PR fixes literally nothing. Stale changes xD |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
RTP packets from Discord may carry tail padding. When the
Pbit is set in the RTP header, the last octet of the payload holds the padding length, including itself (RFC 3550 §5.1). py-cord already parses that bit intoRTPPacket.padding(discord/voice/packets/rtp.py:100) but never acts on it.That was harmless as long as the payload went straight to the Opus decoder — Opus tolerates trailing garbage. It is not harmless with DAVE: the E2EE layer looks for its frame marker at the tail of the payload, so padding buries it and
DaveSession.decryptraisesDecryptionFailed(UnencryptedWhenPassthroughDisabled).The failure mode is nasty because it is quiet.
PacketDecryptor.decrypt_rtpcatches the exception, logs atdebug, and substitutesOPUS_SILENCE. So the symptom users report is not "decryption failed" — it is choppy, stuttering incoming audio, with a large share of frames silently replaced by silence.The fix
Strip the padding before the payload is handed to
dave.decrypt, and only when the P bit is set and the declared length is in range. An out-of-range length means the P bit can't be trusted, so the payload is passed through untouched — truncating a valid frame is worse than leaving a bad one alone.The helper deliberately does not try to detect padding heuristically (e.g. "the tail is a run of identical bytes"). That would corrupt legitimate ciphertext that happens to look that way, and a corrupted frame is indistinguishable from a dropped one in the logs.
Prior art
discord.js hit exactly this bug and fixed it the same way in discordjs/discord.js#11449 (merged 2026-03-13), which closed #11419 and several linked issues. Their check is the same shape:
The Python side has the same bug and never got the same fix. snazzah/davey#15 is a report of this symptom filed against
daveywhile using py-cord ("Most DAVE-encrypted audio packets fail to decrypt (~95% failure rate)"). It was correctly closed with "Thedaveypackage is not the issue here but the implementation of it in whatever library it is being used in." — the reporter's own diagnostic dump has the fingerprint in it (last4=05050505,last4=0f0f0f0f: runs of identical bytes whose value equals the run length), it just wasn't recognised as padding at the time.Verification
Measured on a live voice channel with DAVE enabled, receiving from a desktop client, before and after this patch:
A later run on the same build logged 255 successful frames with 0 failures, of which 3 were packets that actually had the P bit set and went through this code path — those are the frames that would have failed before. Audio quality went from audibly stuttering to clean.
Notes