PR #25714 opened by Kilian (lusoris) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25714 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25714.patch
A failed or short write of an RTSP request now closes the control connection, so later commands and interleaved packets are not sent on a desynchronized stream. This was noted as a pre-existing issue in the automated review of PR 25618. It is stacked on PR 25618 and should land after it; the first two commits here are that PR's commits. Once the request header has been accepted and the body write then fails, the peer still waits for the promised bytes. Before this change the next command or interleaved RTP packet was written into that gap and parsed as part of the old request. **Changes** - `ff_rtsp_send_cmd_with_content_async()` closes both control handles (plain or tunneled) when a header or body write fails or is short. - Reply reads, interleaved TCP reads, TCP muxer writes and `rtsp_write_packet()` return `AVERROR(ENOTCONN)` once the connection is closed, since the handles can now be NULL. - A failed keepalive is now returned from the demuxer's read path. Before, it was only logged; with the connection closed, the server would drop the session anyway. - Teardown and close stay safe when the handles are already NULL. **Tests** - A reproducer with an injected write error: before, the second command returned 0 and wrote 68 bytes on the same connection; after, it returns `ENOTCONN`, writes nothing, and both handles are NULL. The same result holds under AddressSanitizer. - `fate-rtsp`, `fate-rtpdec` and `fate-srtp` pass; `tools/patcheck` reports nothing beyond the changelog note. - The serialization test from the companion test PR passes on top of this change under AddressSanitizer. >From 28a39f28ceb1efe2ac2e8d4e137396788d9cbaa1 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 15:22:03 +0100 Subject: [PATCH 1/3] avformat/rtsp: check command write errors before using replies A failed command write returns before reading the reply. Ignoring that error can make callers inspect an uninitialised reply, including when a request is rejected before it is sent. Return header and body write errors and check command results before using replies in setup, playback, subscription and publishing paths. Keep keepalive failures nonfatal: warn and advance the attempt time so the next packet does not immediately retry the failed request. Map non-OK RECORD replies to invalid data rather than passing an internal -1 error to the caller. Propagate failures when resuming after a subscription and while falling back from UDP to TCP. Injected ENOSPC and EPIPE now propagate without a reply read. Ten reply caller cases and both keepalive methods fail without the caller changes and pass with them. A controlled clock verifies five packet reads, one warning and no retry before timeout/2, with the reply queue unchanged. The loopback RTP session and RTSP, RTP and SRTP FATE tests pass. Signed-off-by: lusoris <[email protected]> --- libavformat/rtsp.c | 15 +++++++++---- libavformat/rtspdec.c | 51 +++++++++++++++++++++++++++++-------------- libavformat/rtspenc.c | 20 ++++++++++------- 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 29f219c366a..d912eedb7be 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1478,6 +1478,7 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, RTSPState *rt = s->priv_data; char buf[MAX_URL_SIZE], *out_buf; char base64buf[AV_BASE64_SIZE(sizeof(buf))]; + int ret; if (!rt->rtsp_hd_out) return AVERROR(ENOTCONN); @@ -1513,13 +1514,15 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, av_log(s, AV_LOG_TRACE, "Sending:\n%s--\n", buf); - ffurl_write(rt->rtsp_hd_out, out_buf, strlen(out_buf)); + if ((ret = ffurl_write(rt->rtsp_hd_out, out_buf, strlen(out_buf))) < 0) + return ret; if (send_content_length > 0 && send_content) { if (rt->control_transport == RTSP_MODE_TUNNEL) { avpriv_report_missing_feature(s, "Tunneling of RTSP requests with content data"); return AVERROR_PATCHWELCOME; } - ffurl_write(rt->rtsp_hd_out, send_content, send_content_length); + if ((ret = ffurl_write(rt->rtsp_hd_out, send_content, send_content_length)) < 0) + return ret; } rt->last_cmd_time = av_gettime_relative(); @@ -1774,7 +1777,9 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port, "RealChallenge2: %s, sd=%s\r\n", rt->session_id, real_res, real_csum); } - ff_rtsp_send_cmd(s, "SETUP", rtsp_st->control_url, cmd, reply, NULL); + err = ff_rtsp_send_cmd(s, "SETUP", rtsp_st->control_url, cmd, reply, NULL); + if (err < 0) + goto fail; if (reply->status_code == 461 /* Unsupported protocol */ && i == 0) { err = 1; goto fail; @@ -2190,7 +2195,9 @@ redirect: "CompanyID: KnKV4M4I/B2FjJ1TToLycw==\r\n" "GUID: 00000000-0000-0000-0000-000000000000\r\n", sizeof(cmd)); - ff_rtsp_send_cmd(s, "OPTIONS", rt->control_uri, cmd, reply, NULL); + err = ff_rtsp_send_cmd(s, "OPTIONS", rt->control_uri, cmd, reply, NULL); + if (err < 0) + goto fail; if (reply->status_code != RTSP_STATUS_OK) { err = ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); goto fail; diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c index 0e592eebaed..0c7e9098b39 100644 --- a/libavformat/rtspdec.c +++ b/libavformat/rtspdec.c @@ -542,7 +542,7 @@ static int rtsp_read_play(AVFormatContext *s) { RTSPState *rt = s->priv_data; RTSPMessageHeader reply1, *reply = &reply1; - int i; + int i, ret; char cmd[MAX_URL_SIZE]; av_log(s, AV_LOG_DEBUG, "hello state=%d\n", rt->state); @@ -584,7 +584,8 @@ static int rtsp_read_play(AVFormatContext *s) rt->seek_timestamp / AV_TIME_BASE, rt->seek_timestamp / (AV_TIME_BASE / 1000) % 1000); } - ff_rtsp_send_cmd(s, "PLAY", rt->control_uri, cmd, reply, NULL); + if ((ret = ff_rtsp_send_cmd(s, "PLAY", rt->control_uri, cmd, reply, NULL)) < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) { return ff_rtsp_averror(reply->status_code, -1); } @@ -613,11 +614,13 @@ static int rtsp_read_pause(AVFormatContext *s) { RTSPState *rt = s->priv_data; RTSPMessageHeader reply1, *reply = &reply1; + int ret; if (rt->state != RTSP_STATE_STREAMING) return 0; else if (!(rt->server_type == RTSP_SERVER_REAL && rt->need_subscription)) { - ff_rtsp_send_cmd(s, "PAUSE", rt->control_uri, NULL, reply, NULL); + if ((ret = ff_rtsp_send_cmd(s, "PAUSE", rt->control_uri, NULL, reply, NULL)) < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) { return ff_rtsp_averror(reply->status_code, -1); } @@ -751,7 +754,11 @@ int ff_rtsp_setup_input_streams(AVFormatContext *s, RTSPMessageHeader *reply) "Require: com.real.retain-entity-for-setup\r\n", sizeof(cmd)); } - ff_rtsp_send_cmd(s, "DESCRIBE", rt->control_uri, cmd, reply, &content); + ret = ff_rtsp_send_cmd(s, "DESCRIBE", rt->control_uri, cmd, reply, &content); + if (ret < 0) { + av_freep(&content); + return ret; + } if (reply->status_code != RTSP_STATUS_OK) { av_freep(&content); return ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); @@ -987,8 +994,10 @@ retry: snprintf(cmd, sizeof(cmd), "Unsubscribe: %s\r\n", rt->last_subscription); - ff_rtsp_send_cmd(s, "SET_PARAMETER", rt->control_uri, - cmd, reply, NULL); + ret = ff_rtsp_send_cmd(s, "SET_PARAMETER", rt->control_uri, + cmd, reply, NULL); + if (ret < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) return ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); rt->need_subscription = 1; @@ -1022,14 +1031,19 @@ retry: } } av_strlcatf(cmd, sizeof(cmd), "%s\r\n", rt->last_subscription); - ff_rtsp_send_cmd(s, "SET_PARAMETER", rt->control_uri, - cmd, reply, NULL); + ret = ff_rtsp_send_cmd(s, "SET_PARAMETER", rt->control_uri, + cmd, reply, NULL); + if (ret < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) return ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); rt->need_subscription = 0; - if (rt->state == RTSP_STATE_STREAMING) - rtsp_read_play (s); + if (rt->state == RTSP_STATE_STREAMING) { + ret = rtsp_read_play(s); + if (ret < 0) + return ret; + } } } @@ -1040,8 +1054,8 @@ retry: rt->lower_transport_mask & (1 << RTSP_LOWER_TRANSPORT_TCP)) { RTSPMessageHeader reply1, *reply = &reply1; av_log(s, AV_LOG_WARNING, "UDP timeout, retrying with TCP\n"); - if (rtsp_read_pause(s) != 0) - return -1; + if ((ret = rtsp_read_pause(s)) != 0) + return ret; // TEARDOWN is required on Real-RTSP, but might make // other servers close the connection. if (rt->server_type == RTSP_SERVER_REAL) @@ -1051,8 +1065,8 @@ retry: if (resetup_tcp(s) == 0) { rt->state = RTSP_STATE_IDLE; rt->need_subscription = 1; - if (rtsp_read_play(s) != 0) - return -1; + if ((ret = rtsp_read_play(s)) != 0) + return ret; goto retry; } } @@ -1068,9 +1082,14 @@ retry: if (rt->server_type == RTSP_SERVER_WMS || (rt->server_type != RTSP_SERVER_REAL && rt->get_parameter_supported)) { - ff_rtsp_send_cmd_async(s, "GET_PARAMETER", rt->control_uri, NULL); + ret = ff_rtsp_send_cmd_async(s, "GET_PARAMETER", rt->control_uri, NULL); } else { - ff_rtsp_send_cmd_async(s, "OPTIONS", rt->control_uri, NULL); + ret = ff_rtsp_send_cmd_async(s, "OPTIONS", rt->control_uri, NULL); + } + if (ret < 0) { + av_log(s, AV_LOG_WARNING, "Failed to send RTSP keepalive: %s\n", + av_err2str(ret)); + rt->last_cmd_time = av_gettime_relative(); } /* The stale flag should be reset when creating the auth response in * ff_rtsp_send_cmd_async, but reset it here just in case we never diff --git a/libavformat/rtspenc.c b/libavformat/rtspenc.c index f2f4b6d5258..370260a0eb8 100644 --- a/libavformat/rtspenc.c +++ b/libavformat/rtspenc.c @@ -48,7 +48,7 @@ int ff_rtsp_setup_output_streams(AVFormatContext *s, const char *addr) { RTSPState *rt = s->priv_data; RTSPMessageHeader reply1, *reply = &reply1; - int i; + int i, ret; char *sdp; AVFormatContext sdp_ctx, *ctx_array[1]; char url[MAX_URL_SIZE]; @@ -82,10 +82,12 @@ int ff_rtsp_setup_output_streams(AVFormatContext *s, const char *addr) return AVERROR_INVALIDDATA; } av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", sdp); - ff_rtsp_send_cmd_with_content(s, "ANNOUNCE", rt->control_uri, - "Content-Type: application/sdp\r\n", - reply, NULL, sdp, strlen(sdp)); + ret = ff_rtsp_send_cmd_with_content(s, "ANNOUNCE", rt->control_uri, + "Content-Type: application/sdp\r\n", + reply, NULL, sdp, strlen(sdp)); av_free(sdp); + if (ret < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) return ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); @@ -114,12 +116,14 @@ static int rtsp_write_record(AVFormatContext *s) RTSPState *rt = s->priv_data; RTSPMessageHeader reply1, *reply = &reply1; char cmd[MAX_URL_SIZE]; + int ret; snprintf(cmd, sizeof(cmd), "Range: npt=0.000-\r\n"); - ff_rtsp_send_cmd(s, "RECORD", rt->control_uri, cmd, reply, NULL); + if ((ret = ff_rtsp_send_cmd(s, "RECORD", rt->control_uri, cmd, reply, NULL)) < 0) + return ret; if (reply->status_code != RTSP_STATUS_OK) - return ff_rtsp_averror(reply->status_code, -1); + return ff_rtsp_averror(reply->status_code, AVERROR_INVALIDDATA); rt->state = RTSP_STATE_STREAMING; return 0; } @@ -132,10 +136,10 @@ static int rtsp_write_header(AVFormatContext *s) if (ret) return ret; - if (rtsp_write_record(s) < 0) { + if ((ret = rtsp_write_record(s)) < 0) { ff_rtsp_close_streams(s); ff_rtsp_close_connections(s); - return AVERROR_INVALIDDATA; + return ret; } return 0; } -- 2.54.0 >From b7419f19531d5bedd17cb8cae5dd24f834b4064d Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 15:23:53 +0100 Subject: [PATCH 2/3] avformat/rtsp: reject truncated command requests Unchecked appends can fill the fixed request buffer before the header terminator is added, sending an incomplete request to the server. Build the request with a bounded AVBPrint and reject truncation before writing anything or advancing CSeq. The preceding caller checks handle this rejection before inspecting a reply that has not been read. Restore Digest authentication state when a too-long request is rejected, so the request has no authentication-side effects. Reject tunneled request bodies before building or writing a header, advancing CSeq, or changing authentication state. A loopback ffprobe test with a 4040-byte SDP control URL sent a 4095-byte SETUP without CRLFCRLF before this change. It now logs an error and sends no SETUP bytes. Direct checks cover overflowing headers, authentication, content length, tunneling and the exact-fit boundary. The loopback RTP session and RTSP, RTP and SRTP FATE tests pass. Signed-off-by: lusoris <[email protected]> --- libavformat/rtsp.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index d912eedb7be..aa85b68cf87 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1478,33 +1478,50 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, RTSPState *rt = s->priv_data; char buf[MAX_URL_SIZE], *out_buf; char base64buf[AV_BASE64_SIZE(sizeof(buf))]; + AVBPrint request; + HTTPAuthState auth_saved; int ret; if (!rt->rtsp_hd_out) return AVERROR(ENOTCONN); + if (rt->control_transport == RTSP_MODE_TUNNEL && send_content_length > 0 && + send_content) { + avpriv_report_missing_feature(s, "Tunneling of RTSP requests with content data"); + return AVERROR_PATCHWELCOME; + } /* Add in RTSP headers */ out_buf = buf; - rt->seq++; - snprintf(buf, sizeof(buf), "%s %s RTSP/1.0\r\n", method, url); + if (rt->auth[0]) + auth_saved = rt->auth_state; + av_bprint_init_for_buffer(&request, buf, sizeof(buf)); + av_bprintf(&request, "%s %s RTSP/1.0\r\n", method, url); if (headers) - av_strlcat(buf, headers, sizeof(buf)); - av_strlcatf(buf, sizeof(buf), "CSeq: %d\r\n", rt->seq); - av_strlcatf(buf, sizeof(buf), "User-Agent: %s\r\n", rt->user_agent); + av_bprintf(&request, "%s", headers); + av_bprintf(&request, "CSeq: %d\r\n", rt->seq + 1); + av_bprintf(&request, "User-Agent: %s\r\n", rt->user_agent); if (rt->session_id[0] != '\0' && (!headers || !strstr(headers, "\nIf-Match:"))) { - av_strlcatf(buf, sizeof(buf), "Session: %s\r\n", rt->session_id); + av_bprintf(&request, "Session: %s\r\n", rt->session_id); } if (rt->auth[0]) { char *str = ff_http_auth_create_response(&rt->auth_state, rt->auth, url, method); if (str) - av_strlcat(buf, str, sizeof(buf)); + av_bprintf(&request, "%s", str); av_free(str); } if (send_content_length > 0 && send_content) - av_strlcatf(buf, sizeof(buf), "Content-Length: %d\r\n", send_content_length); - av_strlcat(buf, "\r\n", sizeof(buf)); + av_bprintf(&request, "Content-Length: %d\r\n", send_content_length); + av_bprintf(&request, "\r\n"); + + if (!av_bprint_is_complete(&request)) { + if (rt->auth[0]) + rt->auth_state = auth_saved; + av_log(s, AV_LOG_ERROR, "RTSP request too long\n"); + return AVERROR(EINVAL); + } + rt->seq++; /* base64 encode rtsp if tunneling */ if (rt->control_transport == RTSP_MODE_TUNNEL) { @@ -1517,10 +1534,6 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, if ((ret = ffurl_write(rt->rtsp_hd_out, out_buf, strlen(out_buf))) < 0) return ret; if (send_content_length > 0 && send_content) { - if (rt->control_transport == RTSP_MODE_TUNNEL) { - avpriv_report_missing_feature(s, "Tunneling of RTSP requests with content data"); - return AVERROR_PATCHWELCOME; - } if ((ret = ffurl_write(rt->rtsp_hd_out, send_content, send_content_length)) < 0) return ret; } -- 2.54.0 >From 20dcd8f255f26891f1204e26f9cd0767ab0565cf Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Tue, 3 Nov 2026 13:54:43 +0100 Subject: [PATCH 3/3] avformat/rtsp: invalidate the control connection after a failed write A failed RTSP request write can leave the peer waiting for bytes promised by an already accepted header. Close both control handles on a short or failed header or body write so no later command or interleaved packet can be sent on the desynchronized connection. Return ENOTCONN from control read and TCP mux paths after invalidation, and propagate an unsuccessful keepalive to the caller. The close paths remain best-effort and safe when the plain or tunneled handles are null. Signed-off-by: lusoris <[email protected]> --- libavformat/rtsp.c | 24 ++++++++++++++++++++++-- libavformat/rtspdec.c | 5 +++++ libavformat/rtspenc.c | 9 ++++++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index aa85b68cf87..86c5eb661cb 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1218,6 +1218,9 @@ int ff_rtsp_skip_packet(AVFormatContext *s) int ret, len, len1; uint8_t buf[MAX_URL_SIZE]; + if (!rt->rtsp_hd) + return AVERROR(ENOTCONN); + rt->pending_packet = 0; ret = ffurl_read_complete(rt->rtsp_hd, buf, 3); if (ret != 3) @@ -1413,6 +1416,9 @@ int ff_rtsp_read_reply(AVFormatContext *s, RTSPMessageHeader *reply, int ret; RTSPState *rt = s->priv_data; + if (!rt->rtsp_hd) + return AVERROR(ENOTCONN); + // If we returned on pending packet last time, // do not try to read again, as it would corrupt // the state due to the already consumed '$'. @@ -1456,6 +1462,20 @@ int ff_rtsp_read_reply(AVFormatContext *s, RTSPMessageHeader *reply, return_on_interleaved_data, method); } +static int rtsp_write_request(AVFormatContext *s, const uint8_t *buf, int size) +{ + RTSPState *rt = s->priv_data; + int ret = ffurl_write(rt->rtsp_hd_out, buf, size); + + if (ret == size) + return 0; + + if (ret >= 0) + ret = AVERROR(EIO); + ff_rtsp_close_connections(s); + return ret; +} + /** * Send a command to the RTSP server without waiting for the reply. * @@ -1531,10 +1551,10 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, av_log(s, AV_LOG_TRACE, "Sending:\n%s--\n", buf); - if ((ret = ffurl_write(rt->rtsp_hd_out, out_buf, strlen(out_buf))) < 0) + if ((ret = rtsp_write_request(s, out_buf, strlen(out_buf))) < 0) return ret; if (send_content_length > 0 && send_content) { - if ((ret = ffurl_write(rt->rtsp_hd_out, send_content, send_content_length)) < 0) + if ((ret = rtsp_write_request(s, send_content, send_content_length)) < 0) return ret; } rt->last_cmd_time = av_gettime_relative(); diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c index 0c7e9098b39..5249da078e9 100644 --- a/libavformat/rtspdec.c +++ b/libavformat/rtspdec.c @@ -917,6 +917,9 @@ int ff_rtsp_tcp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st, int id, len, i, ret; RTSPStream *rtsp_st; + if (!rt->rtsp_hd) + return AVERROR(ENOTCONN); + av_log(s, AV_LOG_TRACE, "tcp_read_packet:\n"); redo: for (;;) { @@ -1095,6 +1098,8 @@ retry: * ff_rtsp_send_cmd_async, but reset it here just in case we never * called the auth code (if we didn't have any credentials set). */ rt->auth_state.stale = 0; + if (ret < 0) + return ret; } } diff --git a/libavformat/rtspenc.c b/libavformat/rtspenc.c index 370260a0eb8..59de5416334 100644 --- a/libavformat/rtspenc.c +++ b/libavformat/rtspenc.c @@ -152,6 +152,9 @@ int ff_rtsp_tcp_write_packet(AVFormatContext *s, RTSPStream *rtsp_st) int size; uint8_t *interleave_header, *interleaved_packet; + if (!rt->rtsp_hd_out) + return AVERROR(ENOTCONN); + size = avio_close_dyn_buf(rtpctx->pb, &buf); rtpctx->pb = NULL; ptr = buf; @@ -188,10 +191,14 @@ static int rtsp_write_packet(AVFormatContext *s, AVPacket *pkt) RTSPState *rt = s->priv_data; RTSPStream *rtsp_st; int n; - struct pollfd p = {ffurl_get_file_handle(rt->rtsp_hd), POLLIN, 0}; + struct pollfd p; AVFormatContext *rtpctx; int ret; + if (!rt->rtsp_hd || !rt->rtsp_hd_out) + return AVERROR(ENOTCONN); + p = (struct pollfd){ffurl_get_file_handle(rt->rtsp_hd), POLLIN, 0}; + while (1) { n = poll(&p, 1, 0); if (n <= 0) -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
