AmAudio::decode: bound decoded PCM size against back buffer (remote heap overflow)#554
Open
hecko wants to merge 1 commit into
Open
AmAudio::decode: bound decoded PCM size against back buffer (remote heap overflow)#554hecko wants to merge 1 commit into
hecko wants to merge 1 commit into
Conversation
The decoder writes its PCM16 output into samples.back_buffer(), a fixed AUDIO_BUFFER_SIZE (8192-byte) half of the DblBuffer. decode() passed the encoded RTP payload size straight to codec->decode() with no check on how much PCM the codec would produce. AmRtpAudio::receive() reads up to AUDIO_BUFFER_SIZE bytes of RTP payload and hands it to decode(). An expanding codec (GSM ~10x, G.722 4x) fed a large payload emits far more than AUDIO_BUFFER_SIZE bytes of PCM, writing past the end of the DblBuffer -- a remotely-triggerable heap overflow on an established call. Predict the decoded sample count via bytes2samples() and reject the frame when the resulting PCM size would exceed the buffer, before the codec runs. Backported from yeti-switch/sems, which added the same pre-decode buffer-size guard in AmAudio::decode(). sems-server lacks yeti's optional codec frames2samples() hook, so this uses the existing bytes2samples() path (equivalent to yeti's fallback).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a defensive guard in AmAudio::decode() to prevent decoders from writing more PCM16 output into the fixed-size DblBuffer back buffer than it can hold, addressing a remotely reachable buffer overflow risk when decoding oversized RTP payloads with expanding codecs.
Changes:
- Add a pre-decode output-size check using
bytes2samples()to reject frames whose decoded PCM16 would exceedAUDIO_BUFFER_SIZE. - Add detailed in-code commentary explaining the overflow scenario and rationale for the guard.
- Minor whitespace cleanup near the end of
decode().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+398
to
+403
| unsigned int out_size = PCM16_S2B(bytes2samples(size)); | ||
| if(out_size > AUDIO_BUFFER_SIZE){ | ||
| ERROR("decoded buffer size for pcm16 (%u) exceeds allowed (%u)\n", | ||
| out_size, AUDIO_BUFFER_SIZE); | ||
| return -1; | ||
| } |
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
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.
Bug
AmAudio::decode()(core/AmAudio.cpp) passes the encoded RTP payload size straight tocodec->decode()without checking how much PCM the codec will produce:The decoder writes its PCM16 output into
samples.back_buffer(), which is a fixedAUDIO_BUFFER_SIZE(8192-byte) half of theDblBuffer(unsigned char samples[AUDIO_BUFFER_SIZE * 2], core/AmAudio.h).AmRtpAudio::receive()reads up toAUDIO_BUFFER_SIZEbytes of RTP payload and hands it todecode(). An expanding codec — GSM (~10×), G.722 (4×) — fed a large payload emits far more thanAUDIO_BUFFER_SIZEbytes of PCM and writes past the end of theDblBuffer. This is a remotely-triggerable heap buffer overflow reachable on an established call by a peer sending an oversized codec payload.Fix
Predict the decoded sample count with
bytes2samples()and reject the frame when the resulting PCM size would exceed the buffer, before invoking the codec:Normal RTP frames (≤30 ms) decode to well under 8192 bytes, so legitimate traffic is unaffected; only abnormally large payloads are dropped.
Provenance
Backported from yeti-switch/sems, which added the same pre-decode buffer-size guard in
AmAudio::decode(). sems-server does not have yeti's optional codecframes2samples()hook, so this uses the existingbytes2samples()path — equivalent to yeti's fallback branch. Thanks to the yeti-switch maintainers for the fix.Generated by Claude Code