fix(rtsp): bound interleaved frame copies - #590
Conversation
8846e22 to
3f29b22
Compare
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-590.eastasia.1.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://thankful-water-0a297bf00-590.eastasia.1.azurestaticapps.net |
There was a problem hiding this comment.
Pull request overview
Fixes a potential overflow in the RTSP TCP interleaved (RTP over RTSP) receive path by validating incoming interleaved frame lengths against the actual zero-copy packet buffer capacity used for forwarding, rather than the larger RTSP response parsing buffer.
Changes:
- Add
buffer_pool.hdependency to accessBUFFER_POOL_BUFFER_SIZEfor bounds checking. - Update interleaved frame length sanity check to reject/resync when
packet_length > BUFFER_POOL_BUFFER_SIZE, preventing oversized frames from reaching thememcpyinto pooled buffers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f29b222fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /* Sanity check: prevent processing packets that are too large */ | ||
| if (packet_length > RTSP_RESPONSE_BUFFER_SIZE - 4) { | ||
| /* Sanity check: bound against the zero-copy destination buffer. */ | ||
| if (packet_length > BUFFER_POOL_BUFFER_SIZE) { |
There was a problem hiding this comment.
When a complete interleaved frame is valid but larger than the 1536-byte pool buffer, this new bound routes it through the generic resync path. That path scans from response_buffer + 1, so an ordinary $ byte inside the oversized RTP/MP2T payload can be mistaken for the next frame marker, leaving the parser aligned to payload bytes and corrupting or dropping subsequent frames. Since the header length is already known after the completeness check, the oversized frame should be consumed by exactly 4 + packet_length bytes rather than resynchronizing inside its payload.
Useful? React with 👍 / 👎.
Summary
This fixes an RTSP TCP interleaved path overflow where the incoming frame length was checked against the 4096-byte RTSP response buffer instead of the 1536-byte zero-copy destination buffer used for RTP forwarding.
Fix
Oversized interleaved frames are now bounded by
BUFFER_POOL_BUFFER_SIZEbefore copying into a pooled packet buffer. Frames above that limit continue through the existing reject/resync path instead of reachingmemcpy.Validation
Built the daemon successfully, ran the RTSP transport e2e coverage, checked e2e collection, and verified the advisory-sized oversized frame with a temporary ASan PoC. The temporary regression check is intentionally not committed.