PR #25618 opened by Kilian (lusoris) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25618 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25618.patch
RTSP requests longer than the 4096-byte command buffer are sent truncated, and write failures on the control connection are ignored. This fixes both, in two commits. - **Truncated requests:** the sender builds the request with unchecked appends. With a 4040-byte SDP control URL, FFmpeg sent a 4095-byte SETUP with no header terminator. The request is now built with a bounded AVBPrint and rejected before anything is written or CSeq advances. - **Write errors:** `ff_rtsp_send_cmd_with_content_async` dropped both `ffurl_write` results, and callers then read a reply that was never received. It now returns those errors, and callers that inspect the reply check them first. This commit comes first so that no commit leaves callers reading an unset reply after a rejected request. TEARDOWN stays best effort, so a dead server does not fail `av_write_trailer`. A failed keepalive logs a warning and waits for the next interval instead of ending the stream. **Tests** - A loopback ffprobe reproducer with a 4040-byte control URL: SETUP was 4095 bytes without CRLFCRLF on base; now no SETUP bytes are sent and an error is logged. A 40-byte control URL still produces a normal 143-byte SETUP. - Direct checks with injected ENOSPC and EPIPE, ten reply-caller cases and the keepalive path fail without the changes and pass with them. A controlled clock shows five packet reads, one warning and no retry before timeout/2. - `fate-rtsp fate-rtpdec fate-srtp` pass on each commit (nine tests, minimal build). This branch and PR 25476 both change `libavformat/rtsp.c` and conflict textually. Whichever lands second needs a small rebase, which I will do on this branch if this one is second. This addresses a pre-existing issue listed in the automated review of PR 25476; it does not change that PR. >From 398cabb634e46807f7eda8a74b484af2af180b7f Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 15:22:03 +0100 Subject: [PATCH 1/2] 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. 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 | 36 ++++++++++++++++++++++++++---------- libavformat/rtspenc.c | 18 +++++++++++------- 3 files changed, 48 insertions(+), 21 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..b1a4c49c627 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,8 +1031,10 @@ 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; @@ -1068,9 +1079,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..6634b2121b2 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,10 +116,12 @@ 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); rt->state = RTSP_STATE_STREAMING; @@ -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 5cbb3d56053c7992f25e2e7ae5e37b2a5d0b9cc3 Mon Sep 17 00:00:00 2001 From: lusoris <[email protected]> Date: Fri, 30 Oct 2026 15:23:53 +0100 Subject: [PATCH 2/2] 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. 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 | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index d912eedb7be..c8f3140f4b0 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))]; + AVBPrint request; int ret; if (!rt->rtsp_hd_out) @@ -1485,26 +1486,32 @@ int ff_rtsp_send_cmd_with_content_async(AVFormatContext *s, /* Add in RTSP headers */ out_buf = buf; - rt->seq++; - snprintf(buf, sizeof(buf), "%s %s RTSP/1.0\r\n", method, url); + 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)) { + 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) { -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
