PR #24961 opened by furkanefecan
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24961
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24961.patch
Both of these came out of running FATE under AddressSanitizer and
UndefinedBehaviorSanitizer. They are independent, so feel free to take them
separately.
### 1/2 — avformat/rtsp: Validate the SampleRate SDP attribute
`sdp_parse_line()` takes the `SampleRate:integer;` attribute with `atoi()` and
stores it straight into `codecpar->sample_rate` with no check, so a negative or
out-of-range value from the SDP ends up on the stream.
Measured against the sdp demuxer with the rtpmap rate held at 44100 and only the
`SampleRate` attribute varied, so it is unambiguous which code path sets the
value:
| SDP | before | after |
| --- | --- | --- |
| no `SampleRate` line | 44100 Hz | 44100 Hz |
| `SampleRate:integer;48000` | 48000 Hz | 48000 Hz |
| `SampleRate:integer;-1` | **-1 Hz** | rejected, keeps 44100 |
| `SampleRate:integer;99999999999999999999` | **-1 Hz** | rejected, keeps 44100
|
The second row is the positive control: seeing 48000 proves this attribute
really
is what sets the rate, rather than the rtpmap line.
### 2/2 — avformat/mov: Check sample_rate in stsd version 2 audio
The QuickTime SoundDescription version 2 layout stores the sample rate as a 64
bit
float, and `mov_parse_stsd_audio()` converts it to `int` with no validation at
all.
That conversion is undefined behaviour for NaN and for out-of-range values:
```
libavformat/mov.c:3031:41: runtime error: nan is outside the range of
representable values of type 'int'
#0 mov_parse_stsd_audio libavformat/mov.c:3031
#1 ff_mov_read_stsd_entries libavformat/mov.c:3402
#2 mov_read_stsd libavformat/mov.c:3490
#7 mov_read_trak libavformat/mov.c:5573
```
The `sample_rate < 0` check in `ff_mov_read_stsd_entries()` runs *after* the
conversion, so it cannot prevent this. NaN also defeats integer-style guards
because every comparison against it is false, which is the same reason soxdec
needed an explicit `isnan()` check in d2d79dca9a.
A valid file (`sample_rate = 48000.0`) reports 48000 Hz before and after the
patch.
> Note for reproducing 2/2: GCC's `-fsanitize=undefined` does **not** include
> `float-cast-overflow` (unlike clang), so this is invisible unless that check
> is
> enabled explicitly.
### Testing
Both were verified with a positive control first — a valid input that has to
keep
producing the same result — before the malformed input was tried.
FATE with both applied: **2958 tests, 0 failures.**
---
These patches were written with AI assistance, noted per-commit in the
`Co-Authored-By` trailers. Every claim above was produced by running the code.
From 91c043a42deb23744fb921a60d29f98a1145121d Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Wed, 7 Oct 2026 03:23:08 +0300
Subject: [PATCH 1/2] avformat/rtsp: Validate the SampleRate SDP attribute
sdp_parse_line() stored the "SampleRate:integer;" attribute straight into
codecpar->sample_rate without any check:
st = s->streams[s->nb_streams - 1];
st->codecpar->sample_rate = atoi(p);
The value comes from the SDP description, i.e. from the server, so a
negative sample rate ends up on the stream. atoi() additionally has
undefined behaviour when the value does not fit into an int, which is
why 088f08db71 removed it from matroskaenc.
Measured with a minimal SDP fed to the sdp demuxer, before this patch:
SampleRate:integer;48000 -> sample_rate = 48000
SampleRate:integer;-1 -> sample_rate = -1
SampleRate:integer;99999999999999999999 -> sample_rate = -1
Every other demuxer rejects such values (4xm.c, adxdec.c, 3dostr.c,
apac.c, ads.c all test sample_rate <= 0), and rtsp.c itself already
guards the rtpmap sample rate and channel count with "if (i > 0)" a few
hundred lines earlier, so only this attribute was left unchecked.
Use strtol() and ignore values outside the valid range, keeping whatever
rate the rtpmap line established. After the patch the same three inputs
give 48000, 44100 and 44100, with a warning logged for the latter two.
Note this is defence in depth rather than a fix for a reachable crash:
nothing in the RTP/RDT path divides by sample_rate, and avcodec.c
rejects a negative rate when the decoder is opened.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/rtsp.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index cdf7b7c389..68ed5d7680 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -681,8 +681,15 @@ static void sdp_parse_line(AVFormatContext *s,
SDPParseState *s1,
rt->transport = RTSP_TRANSPORT_RDT;
} else if (av_strstart(p, "SampleRate:integer;", &p) &&
s->nb_streams > 0) {
- st = s->streams[s->nb_streams - 1];
- st->codecpar->sample_rate = atoi(p);
+ long sample_rate = strtol(p, NULL, 0);
+
+ if (sample_rate <= 0 || sample_rate > INT_MAX) {
+ av_log(s, AV_LOG_WARNING,
+ "Invalid SampleRate %ld in SDP, ignoring\n",
sample_rate);
+ } else {
+ st = s->streams[s->nb_streams - 1];
+ st->codecpar->sample_rate = sample_rate;
+ }
} else if (av_strstart(p, "crypto:", &p) && s->nb_streams > 0) {
// RFC 4568
rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
--
2.52.0
From f04f6b5c3f163900d4fdd7f5a642eda5679796d1 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Thu, 8 Oct 2026 14:38:08 +0300
Subject: [PATCH 2/2] avformat/mov: Check sample_rate in stsd version 2 audio
The QuickTime SoundDescription version 2 layout stores the sample rate as
a 64 bit float. Converting it to int is undefined behaviour when the value
does not fit into int, NaN included -- and NaN slips through every
integer-style range check because all comparisons against it are false.
The sample_rate < 0 check in ff_mov_read_stsd_entries() runs after the
conversion has already happened, so it cannot prevent this.
Validate the double before converting it, matching soxdec.
Fixes: mov.c:3031 runtime error: nan is outside the range of
representable values of type 'int'
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/mov.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 71c4e38c88..7c5946c0a9 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2987,8 +2987,8 @@ static void mov_parse_stsd_video(MOVContext *c,
AVIOContext *pb,
}
}
-static void mov_parse_stsd_audio(MOVContext *c, AVIOContext *pb,
- AVStream *st, MOVStreamContext *sc)
+static int mov_parse_stsd_audio(MOVContext *c, AVIOContext *pb,
+ AVStream *st, MOVStreamContext *sc)
{
int bits_per_sample, flags;
uint16_t version = avio_rb16(pb);
@@ -3027,8 +3027,18 @@ static void mov_parse_stsd_audio(MOVContext *c,
AVIOContext *pb,
sc->bytes_per_frame = avio_rb32(pb);
avio_rb32(pb); /* bytes per sample */
} else if (version == 2) {
+ double sample_rate;
+
avio_rb32(pb); /* sizeof struct only */
- st->codecpar->sample_rate = av_int2double(avio_rb64(pb));
+ sample_rate = av_int2double(avio_rb64(pb));
+
+ if (sample_rate <= 0 || sample_rate > INT_MAX ||
+ isnan(sample_rate)) {
+ av_log(c->fc, AV_LOG_ERROR, "Invalid sample rate %g\n",
+ sample_rate);
+ return AVERROR_INVALIDDATA;
+ }
+ st->codecpar->sample_rate = sample_rate;
channel_count = avio_rb32(pb);
st->codecpar->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC;
st->codecpar->ch_layout.nb_channels = channel_count;
@@ -3106,6 +3116,8 @@ static void mov_parse_stsd_audio(MOVContext *c,
AVIOContext *pb,
st->codecpar->bits_per_coded_sample = bits_per_sample;
sc->sample_size = (bits_per_sample >> 3) *
st->codecpar->ch_layout.nb_channels;
}
+
+ return 0;
}
static void mov_parse_stsd_subtitle(MOVContext *c, AVIOContext *pb,
@@ -3399,7 +3411,9 @@ int ff_mov_read_stsd_entries(MOVContext *c, AVIOContext
*pb, int entries)
if (st->codecpar->codec_type==AVMEDIA_TYPE_VIDEO) {
mov_parse_stsd_video(c, pb, st, sc);
} else if (st->codecpar->codec_type==AVMEDIA_TYPE_AUDIO) {
- mov_parse_stsd_audio(c, pb, st, sc);
+ ret = mov_parse_stsd_audio(c, pb, st, sc);
+ if (ret < 0)
+ return ret;
if (st->codecpar->sample_rate < 0) {
av_log(c->fc, AV_LOG_ERROR, "Invalid sample rate %d\n",
st->codecpar->sample_rate);
return AVERROR_INVALIDDATA;
--
2.52.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
