Skip to content

fix(security): bound publish/play AMF waits with wall-clock deadline#145

Merged
AlexanderWagnerDev merged 1 commit into
mainfrom
cursor/application-security-review-e08c
Jul 19, 2026
Merged

fix(security): bound publish/play AMF waits with wall-clock deadline#145
AlexanderWagnerDev merged 1 commit into
mainfrom
cursor/application-security-review-e08c

Conversation

@cursor

@cursor cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Security review fix

Fixes #144

Finding (Medium)

Location: src/client/mod.rs

Impact: A malicious RTMP server that completes connect + createStream but withholds onStatus during publish() or play() could block the embedder's calling thread for up to ~650 seconds (64 recv polls × 10s RECV_POLL_TIMEOUT_MS, plus up to 10s on send).

Root cause

do_amf_connect() threads a wall-clock deadline through its AMF exchanges, but publish() and play() passed None, so wait_for_command() fell back to unbounded per-poll 10s waits.

Fix

  • Add command_io_deadline() that reuses the configured connect_timeout (default 10s).
  • Pass Some(deadline) from publish() and play() into send_command_msg() and wait_for_command().
  • Add regression test publish_and_play_honor_command_io_deadline.

Remediation leverage

Single helper + two call-site changes; no API break. set_connect_timeout() now also governs publish/play blocking I/O, matching the existing comment that recv budgets apply during those phases.

Open in Web View Automation 

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Touches blocking client I/O and timeout behavior on publish/play; low API surface but security-relevant thread-blocking fix.

Overview
Fixes a denial-of-service path where a server that stalls after createStream could keep publish() or play() blocked for many minutes because those paths waited for onStatus with no overall deadline.

publish() and play() now compute a wall-clock deadline via new command_io_deadline(), reusing the existing connect_timeout / default 10s budget, and pass it into send_command_msg() and wait_for_command()—matching what do_amf_connect() already did. Docs on connect_timeout and set_connect_timeout() are updated to state that this budget applies to blocking I/O in connect(), publish(), and play().

A regression test asserts both methods return ErrorCode::Timeout near the configured deadline when the peer sends nothing.

Reviewed by Cursor Bugbot for commit 86c1b11. Bugbot is set up for automated code reviews on this repo. Configure here.

Client::publish() and Client::play() passed None as the I/O deadline,
so a malicious server could stall the caller for up to ~650 seconds
(64 recv polls × 10s) after connect succeeded. Reuse the configured
connect_timeout budget for these blocking AMF exchanges and add a
regression test.

Co-authored-by: Alexander Wagner <info@alexanderwagnerdev.com>
@AlexanderWagnerDev
AlexanderWagnerDev marked this pull request as ready for review July 19, 2026 15:53
@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_702b6dcf-faf4-4ec1-b722-1745f9a2a205)

@AlexanderWagnerDev
AlexanderWagnerDev merged commit e25fa1e into main Jul 19, 2026
31 of 32 checks passed
@AlexanderWagnerDev
AlexanderWagnerDev deleted the cursor/application-security-review-e08c branch July 20, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client publish/play lack wall-clock deadline enabling ~650s thread hang

2 participants