PR #25485 opened by furkanefecan
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25485
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25485.patch
> **Stacked on #25181.** The first five commits here are that PR, unchanged and
> already approved; the two new ones are `avformat/rtpdec: Avoid atoi() in the
> SDP parsers` and its tests. #25181 should go in first — I am not asking for
> the approved five to be looked at again. Forgejo cannot base a PR on a branch
> in my fork, which is why they appear here.
## What
Every remaining `atoi()` in the RTP depacketizers' SDP parsing, converted to
the bounded parse #25181 adds. Nine files, eighteen call sites.
## Why this is not only about undefined behaviour
`atoi()` is undefined when the value does not fit an `int`, and where it is
`(int)strtol()` the value wraps. Several of these parsers range check the
attribute **after** converting it, so the check only sees what the conversion
produced — and a value far outside the accepted range can arrive inside it.
Measured on current master, minimal SDP fed to the sdp demuxer:
```
a=fmtp:112 sampling=YCbCr-4:2:2; width=320; height=240; depth=8
-> codec_name=rawvideo width=320 accepted, as intended
a=fmtp:112 ... depth=9
-> codec_name=unknown refused, as intended
a=fmtp:112 ... depth=4294967304
-> codec_name=rawvideo width=320 refused nowhere
```
4294967304 is neither 8 nor 10, so `rfc4175_parse_format()` should refuse it.
It arrives there as 8 and the stream is configured as 8 bit UYVY422 from a
depth the sender never asked for. Same shape elsewhere:
```
width=4294967616 -> arrives as 320, accepted
sprop-maxcapturerate=4294975296 -> arrives as 8000, accepted silently
sprop-maxcapturerate=7999 -> refused, as intended
mode=4294967316 (iLBC) -> arrives as 20, passes the switch
octet-align=4294967297 (AMR)-> arrives as 1, passes !octet_align
```
The SDP description comes from the server, so these are its values, not the
user's.
## Ranges applied
| parser | attribute | range |
| --- | --- | --- |
| rfc4175 | width, height | 1..INT_MAX |
| rfc4175 | depth | 1..INT_MAX, then the existing per-sampling test |
| opus | sprop-maxcapturerate | 8000..48000 (the test it already had) |
| ilbc | mode | 0..INT_MAX, then the existing switch |
| amr | octet-align, crc | 0..1 |
| amr | interleaving | 0..INT_MAX |
| amr | channels | 1..6 |
| h264 | packetization-mode | 0..2 (RFC 6184) |
| hevc | profile-id | 0..31 |
| hevc | sprop-max-don-diff, sprop-depack-buf-nalus | 0..32767 |
| av1 | profile 0..2, level-idx 0..31, tier 0..1 | |
| rdt | StartTime | INT_MIN..INT_MAX |
The AV1 fields are `uint8_t`, so `atoi()` there truncated twice — once into an
`int`, once into the field. `rdt`'s `first_dts` is `int64_t` but `atoi()` never
produced more than an `int`, so this keeps the range it could represent rather
than widening it.
## Tests
Seven new tests, no external sample and no network connection. Measured which
depend on the change, by reverting only its nine source files and keeping the
tests: **three fail, four pass either way.**
The three are the wrapped values. The four are deliberate: two accepted depths
and one accepted rate, so a change that refused everything would be caught
rather than passing, plus `depth=9` — outside the accepted set but inside the
`int` range, so refused before and after. It pairs with the wrapped depth: the
same outcome has to be reached whether the value is out of range directly or
only after wrapping.
`-rfc4175-depth-invalid` reports `width=320` while `-rfc4175-depth-wrapped`
reports `width=0`, because a depth that cannot be converted ends the fmtp line
before the width is applied, whereas 9 converts and is refused later. Both
references record what each path actually does rather than what I expected.
## Verification
* 30 RTP/SDP FATE tests pass: the 23 already in tree or in #25181, plus these 7.
* Each of the two new commits builds and passes on its own.
* Full `--disable-autodetect` FATE run clean.
* `git diff --check` clean, no C line over 80 columns, `tools/patcheck` reports
only its usual changelog hint.
One unrelated thing fixed in passing because it sits on a touched line: the AV1
`level-idx` diagnostic printed `profile` rather than `level_idx`.
Written with AI assistance, noted per-commit in the Co-Authored-By trailers.
Every number above was produced by running the tests.
From a06324623bb60addd46fe02618dee48fdec3e72f Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 12:33:09 +0300
Subject: [PATCH 1/7] avformat: Add ff_parse_bounded_int()
atoi() has undefined behaviour when the value does not fit into an int,
which is why 088f08db71 removed it from matroskaenc. libavformat still
calls it 61 times, and in the demuxers the string it parses comes from a
file or from the network.
The alternative today is a hand-written strtol() call, of which
libavformat has 84, each with its own idea of which failures to check:
some test errno for ERANGE, some test the range, some test neither. The
range check is the part that is easy to forget, and forgetting it is what
lets a value from the input reach codecpar.
Add one helper that does all of it, so that callers state the range they
accept and cannot silently skip checking it:
if (ff_parse_bounded_int(value, 1, 1048561, &size) < 0)
return AVERROR_INVALIDDATA;
strtoll() is used rather than strtol() so that the ERANGE boundary is
beyond the int range on every supported platform, and the range test is
what rejects the value, not the width of long. Base 10 is fixed, because
with base 0 a leading zero would select octal and silently reinterpret
decimal values.
Tested by the callers added in the following commits; fate-rtpdec-sdp
covers in-range, out-of-range, non-numeric and above-INT_MAX input.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/internal.h | 22 ++++++++++++++++++++++
libavformat/utils.c | 21 +++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/libavformat/internal.h b/libavformat/internal.h
index 4db9db85ce5..dd51d8a334c 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -437,6 +437,28 @@ char *ff_data_to_hex(char *buf, const uint8_t *src, int
size, int lowercase);
*/
int ff_hex_to_data(uint8_t *data, const char *p);
+/**
+ * Parse a base 10 integer and check that it fits a given range.
+ *
+ * Leading white space and a leading sign are accepted, parsing stops at the
+ * first character that cannot be part of the number, and *res is only written
+ * when the whole value is in range.
+ *
+ * This exists because atoi() has undefined behaviour when the value does not
+ * fit into an int, which makes it unsuitable for parsing numbers that come
+ * from a file or from the network; see 088f08db71. Demuxers that need to
+ * reject out of range values should use this instead of atoi() or of a
+ * hand-written strtol() call, so that the range check cannot be forgotten.
+ *
+ * @param p the string to parse
+ * @param min smallest accepted value
+ * @param max largest accepted value, must not be smaller than min
+ * @param res the parsed value is written here on success
+ * @return 0 on success, AVERROR(EINVAL) if p does not start with a number,
+ * AVERROR(ERANGE) if the number is outside [min, max]
+ */
+int ff_parse_bounded_int(const char *p, int min, int max, int *res);
+
#define NTP_OFFSET 2208988800ULL
#define NTP_OFFSET_US (NTP_OFFSET * 1000000ULL)
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 4874a825a0a..2a7a25a8aad 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -19,7 +19,9 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <errno.h>
#include <stdint.h>
+#include <stdlib.h>
#include <time.h>
#include "config.h"
@@ -492,6 +494,25 @@ char *ff_data_to_hex(char *buff, const uint8_t *src, int
s, int lowercase)
return buff;
}
+int ff_parse_bounded_int(const char *p, int min, int max, int *res)
+{
+ char *tail;
+ long long v;
+
+ av_assert1(min <= max);
+
+ errno = 0;
+ v = strtoll(p, &tail, 10);
+ if (tail == p)
+ return AVERROR(EINVAL);
+ if (errno == ERANGE || v < min || v > max)
+ return AVERROR(ERANGE);
+
+ *res = v;
+
+ return 0;
+}
+
int ff_hex_to_data(uint8_t *data, const char *p)
{
int c, len, v;
--
2.54.0
From 5787546d15ce5e7b7e650d15abf9d37f3e809d03 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 12:33:24 +0300
Subject: [PATCH 2/7] avformat/rtpdec_h264: Validate the framesize SDP
attribute
ff_h264_parse_framesize() stored the "a=framesize:<pt> <w>-<h>" dimensions
straight into codecpar without any check:
par->width = atoi(buf1);
par->height = atoi(p + 1); // skip the -
The value comes from the SDP description, i.e. from the server, and the
function is used for both H264 and HEVC streams. atoi() additionally has
undefined behaviour when the value does not fit into an int.
Measured with a minimal SDP fed to the sdp demuxer, before this patch,
reading the size back from the imgutils diagnostic because it prints the
stored value:
a=framesize:96 320-240 -> 320x240
a=framesize:96 -1--1 -> 0x0
a=framesize:96 0-0 -> 0x0
a=framesize:96 65536-65536 -> 65536x65536
a=framesize:96 2147483648-2147483648 -> 2147483648x2147483648
a=framesize:96 99999999999999999999-5 -> 4294967295x5
The last three are what reached codecpar: the message prints the value
unsigned, so 4294967295 is -1 and 2147483648 is INT_MIN, both produced by
atoi() on input it cannot represent.
Parse with ff_parse_bounded_int() and apply av_image_check_size(), which
is the limit the rest of FFmpeg enforces and what rtpdec_rfc4175.c already
does for the same kind of attribute. Keep whatever size was there when the
attribute is unusable, and say so, taking a log context for that.
Measured after the patch:
a=framesize:96 320-240 -> 320x240
a=framesize:96 320-240 trailing -> 320x240
a=framesize:96 -1--1 -> rejected, stays unset
a=framesize:96 0-0 -> rejected, stays unset
a=framesize:96 65536-65536 -> rejected, stays unset
a=framesize:96 2147483648-2147483648 -> rejected, stays unset
a=framesize:96 99999999999999999999-5 -> rejected, stays unset
Note this is defence in depth rather than a fix for a reachable crash: the
decoder calls av_image_check_size() as well and refuses the frame, which
is why the probed size was already 0x0 for most of the values above. What
changes is that the value no longer reaches codecpar, and that the
diagnostic names which check refused it.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/rtpdec_formats.h | 3 ++-
libavformat/rtpdec_h264.c | 28 ++++++++++++++++++++++++----
libavformat/rtpdec_hevc.c | 2 +-
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/libavformat/rtpdec_formats.h b/libavformat/rtpdec_formats.h
index 1ff2a72d2ae..259001c70bf 100644
--- a/libavformat/rtpdec_formats.h
+++ b/libavformat/rtpdec_formats.h
@@ -45,7 +45,8 @@ int ff_h264_handle_aggregated_packet(AVFormatContext *ctx,
PayloadContext *data,
int ff_h264_handle_frag_packet(AVPacket *pkt, const uint8_t *buf, int len,
int start_bit, const uint8_t *nal_header,
int nal_header_len);
-void ff_h264_parse_framesize(AVCodecParameters *par, const char *p);
+void ff_h264_parse_framesize(void *logctx, AVCodecParameters *par,
+ const char *p);
extern const RTPDynamicProtocolHandler ff_ac3_dynamic_handler;
extern const RTPDynamicProtocolHandler ff_amr_nb_dynamic_handler;
diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
index c145edaa5e5..c749fab5590 100644
--- a/libavformat/rtpdec_h264.c
+++ b/libavformat/rtpdec_h264.c
@@ -33,12 +33,16 @@
* FU-B packet types)
*/
+#include <limits.h>
+
#include "libavutil/attributes.h"
#include "libavutil/base64.h"
#include "libavutil/intreadwrite.h"
#include "libavutil/avstring.h"
+#include "libavutil/imgutils.h"
#include "libavutil/mem.h"
#include "avformat.h"
+#include "internal.h"
#include "rtpdec.h"
#include "rtpdec_formats.h"
@@ -182,10 +186,13 @@ static int sdp_parse_fmtp_config_h264(AVFormatContext *s,
return 0;
}
-void ff_h264_parse_framesize(AVCodecParameters *par, const char *p)
+void ff_h264_parse_framesize(void *logctx, AVCodecParameters *par,
+ const char *p)
{
char buf1[50];
char *dst = buf1;
+ const char *size_str;
+ int width, height;
// remove the protocol identifier
while (*p && *p == ' ')
@@ -194,6 +201,7 @@ void ff_h264_parse_framesize(AVCodecParameters *par, const
char *p)
p++; // eat protocol identifier
while (*p && *p == ' ')
p++; // strip trailing spaces.
+ size_str = p; // the "320-240" part, for diagnostics
while (*p && *p != '-' && (dst - buf1) < sizeof(buf1) - 1)
*dst++ = *p++;
*dst = '\0';
@@ -202,9 +210,21 @@ void ff_h264_parse_framesize(AVCodecParameters *par, const
char *p)
return;
// a='framesize:96 320-240'
+ // The dimensions come from the SDP description, so they have to be
+ // checked before they reach codecpar: atoi() has undefined behaviour
+ // when the value does not fit into an int, and a negative or oversized
+ // size used to be stored as is.
+ if (ff_parse_bounded_int(buf1, 1, INT_MAX, &width) < 0 ||
+ ff_parse_bounded_int(p + 1, 1, INT_MAX, &height) < 0 ||
+ av_image_check_size(width, height, 0, logctx) < 0) {
+ av_log(logctx, AV_LOG_WARNING,
+ "Invalid framesize %s in SDP, ignoring\n", size_str);
+ return;
+ }
+
// set our parameters
- par->width = atoi(buf1);
- par->height = atoi(p + 1); // skip the -
+ par->width = width;
+ par->height = height;
}
int ff_h264_handle_aggregated_packet(AVFormatContext *ctx, PayloadContext
*data, AVPacket *pkt,
@@ -402,7 +422,7 @@ static int parse_h264_sdp_line(AVFormatContext *s, int
st_index,
stream = s->streams[st_index];
if (av_strstart(p, "framesize:", &p)) {
- ff_h264_parse_framesize(stream->codecpar, p);
+ ff_h264_parse_framesize(s, stream->codecpar, p);
} else if (av_strstart(p, "fmtp:", &p)) {
return ff_parse_fmtp(s, stream, h264_data, p,
sdp_parse_fmtp_config_h264);
} else if (av_strstart(p, "cliprect:", &p)) {
diff --git a/libavformat/rtpdec_hevc.c b/libavformat/rtpdec_hevc.c
index 0e29efc3d60..47da291db27 100644
--- a/libavformat/rtpdec_hevc.c
+++ b/libavformat/rtpdec_hevc.c
@@ -141,7 +141,7 @@ static av_cold int hevc_parse_sdp_line(AVFormatContext
*ctx, int st_index,
par = current_stream->codecpar;
if (av_strstart(sdp_line_ptr, "framesize:", &sdp_line_ptr)) {
- ff_h264_parse_framesize(par, sdp_line_ptr);
+ ff_h264_parse_framesize(ctx, par, sdp_line_ptr);
} else if (av_strstart(sdp_line_ptr, "fmtp:", &sdp_line_ptr)) {
int ret = ff_parse_fmtp(ctx, current_stream, hevc_data, sdp_line_ptr,
hevc_sdp_parse_fmtp_config);
--
2.54.0
From 336b094eb77bcafb26d4dfb6df0c9b7cfa71ee9b Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 16:55:11 +0300
Subject: [PATCH 3/7] avformat/rtpdec_xiph: Validate the Xiph fmtp width and
height
The width and height from the Xiph fmtp line went into codecpar unchecked:
} else if (!strcmp(attr, "width")) {
/* This is an integer between 1 and 1048561
* and MUST be in multiples of 16. */
par->width = atoi(value);
The comment states the range, but nothing enforced it, and atoi() has
undefined behaviour when the value does not fit into an int. Unlike the
framesize attribute handled in the previous commit, nothing downstream
clears these: the value was reported through codecpar exactly as parsed.
Measured with a minimal SDP fed to the sdp demuxer, before this patch:
width=320 -> width=320
width=1048561 -> width=1048561
width=1048562 -> width=1048562
width=-1 -> width=-1
width=2147483648 -> width=-2147483648
width=99999999999999999999 -> width=-1
The last two are atoi() on input it cannot represent. An application
reading AVCodecParameters::width got INT_MIN.
Use ff_parse_bounded_int() with the documented range and refuse the line
otherwise, as rtpdec_rfc4175.c does for its own size attributes. The range
is from section 6 of RTP Payload Format for Theora Encoded Video
(draft-barbato-avt-rtp-theora-01), which is also where the multiple of 16
requirement comes from; that part is deliberately not enforced, streams in
the wild do not always honour it, and the range is what keeps the value
representable.
Refusing the attribute ends the fmtp line, so a configuration= later on
that same line is not parsed. That is the behaviour this function already
has for an unsupported sampling value, and rtpdec_rfc4175.c refuses its
line the same way, so it is the existing policy for a malformed fmtp
rather than a new one.
Measured after the patch:
width=320 -> width=320
width=1048561 -> width=1048561
width=1048562 -> refused
width=-1 -> refused
width=2147483648 -> refused
width=99999999999999999999 -> refused
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/rtpdec_xiph.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/libavformat/rtpdec_xiph.c b/libavformat/rtpdec_xiph.c
index 98e4db48f90..20add09ea05 100644
--- a/libavformat/rtpdec_xiph.c
+++ b/libavformat/rtpdec_xiph.c
@@ -303,15 +303,23 @@ static int xiph_parse_fmtp_pair(AVFormatContext *s,
"Unsupported pixel format %s\n", attr);
return AVERROR_INVALIDDATA;
}
- } else if (!strcmp(attr, "width")) {
- /* This is an integer between 1 and 1048561
- * and MUST be in multiples of 16. */
- par->width = atoi(value);
- return 0;
- } else if (!strcmp(attr, "height")) {
- /* This is an integer between 1 and 1048561
- * and MUST be in multiples of 16. */
- par->height = atoi(value);
+ } else if (!strcmp(attr, "width") || !strcmp(attr, "height")) {
+ /* Section 6 of RTP Payload Format for Theora Encoded Video
+ * (draft-barbato-avt-rtp-theora-01) states an integer between 1 and
+ * 1048561, in multiples of 16. The multiple of 16 is not enforced,
+ * but the range is: the value comes from the SDP description and
+ * used to reach codecpar unchecked through atoi(), whose behaviour is
+ * undefined when the value does not fit into an int. */
+ int size;
+
+ if (ff_parse_bounded_int(value, 1, 1048561, &size) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid %s %s in SDP\n", attr, value);
+ return AVERROR_INVALIDDATA;
+ }
+ if (!strcmp(attr, "width"))
+ par->width = size;
+ else
+ par->height = size;
return 0;
} else if (!strcmp(attr, "delivery-method")) {
/* Possible values are: inline, in_band, out_band/specific_name. */
--
2.54.0
From d13180966a8cda097898f92d65059e12f9179a61 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 16:55:37 +0300
Subject: [PATCH 4/7] tests/fate: Test the SDP dimension attributes
Eleven ffprobe tests for the two attributes validated in the previous
commits, none of which needs an external sample or a network connection:
the descriptions are generated inline and -sdp_flags custom_io keeps the
demuxer from opening a transport. Plus nine cases in the existing RTP unit
test for the side those cannot reach.
Measured which of the eleven depend on the fixes, by restoring libavformat
to 46f23be8b1 while keeping the tests:
| test | without the fixes |
| --- | --- |
| fate-rtpdec-sdp-h264-framesize | passes |
| fate-rtpdec-sdp-h264-framesize-zero | fails |
| fate-rtpdec-sdp-h264-framesize-negative | fails |
| fate-rtpdec-sdp-h264-framesize-above-int-max | fails |
| fate-rtpdec-sdp-h264-framesize-overflow | fails |
| fate-rtpdec-sdp-h264-framesize-too-large | fails |
| fate-rtpdec-sdp-h264-framesize-bad-height | fails |
| fate-rtpdec-sdp-xiph-width-above-max | fails |
| fate-rtpdec-sdp-xiph-width-negative | fails |
| fate-rtpdec-sdp-xiph-width-above-int-max | fails |
| fate-rtpdec-sdp-xiph-width-overflow | fails |
They all compare the diagnostic rather than the probed size, for two
different reasons. For the framesize values the probed size is 0x0 either
way, because the decoder's own av_image_check_size() refused them before
the attribute was checked, so only the diagnostic says which check fired.
For the Xiph ones a stream whose packed header is not a real one cannot be
probed at all where the theora decoder is built in, so there both the
probe output and the exit status depend on the configuration while the
diagnostic does not; those four discard the exit status, as
fate-filter-pan-channel-id-limit and the fate-id3v2-* tests do.
That leaves a gap the ffprobe tests cannot close: they only ever show a
dimension being refused, so an implementation that refused every Xiph
dimension would pass all eleven. Measured, by making the Xiph range check
unconditionally fail: eleven of eleven still pass. The nine cases added to
libavformat/tests/rtpdec call the Theora SDP callback directly, which needs
no decoder and no codec bitstream, and print the return value and the
resulting dimensions; they cover accepted dimensions, both ends of the
range, accepted non-multiples of 16, refused heights and a non-numeric
width, and that same mutation makes them fail.
fate-rtpdec-sdp-h264-framesize is the positive control for the framesize
path. It is not one for the Xiph path, which assigns through different
code; the unit test cases are what cover that side.
fate-rtpdec-sdp-h264-framesize-bad-height is the only case where it is the
height that is refused rather than the width. It is coverage of that side
of the parse and not a discriminating control: measured by removing just
the height's range check, av_image_check_size() refuses the value atoi()
produces as well, so the diagnostic appears either way and no ffprobe test
distinguishes the two.
The ffprobe group is named rtpdec-sdp because fate-rtpdec is already the
libavformat/tests/rtpdec unit test, which the new cases extend.
Measured both ways round: all twelve tests pass both in a build configured
for only the components they need and in a full --disable-autodetect build,
and restoring libavformat to 46f23be8b1 fails eleven of them - the ten in
the table plus fate-rtpdec, whose new cases accept a height of 0 without
the range check.
The unit test cases come from Forgejo_Fairy's review of this series,
commit 7054b65e421b on its fairy/pr25181-xiph-tests branch; I reproduced
its mutation result before taking them.
Co-authored-by: Forgejo_Fairy <[email protected]>
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/tests/rtpdec.c | 49 ++++++++++
tests/Makefile | 1 +
tests/fate/rtpdec-sdp.mak | 115 +++++++++++++++++++++++
tests/ref/fate/rtpdec | 9 ++
tests/ref/fate/rtpdec-sdp-h264-framesize | 1 +
5 files changed, 175 insertions(+)
create mode 100644 tests/fate/rtpdec-sdp.mak
create mode 100644 tests/ref/fate/rtpdec-sdp-h264-framesize
diff --git a/libavformat/tests/rtpdec.c b/libavformat/tests/rtpdec.c
index e3a8d397ac8..3f49e19e4bc 100644
--- a/libavformat/tests/rtpdec.c
+++ b/libavformat/tests/rtpdec.c
@@ -250,6 +250,52 @@ static void dump(const char *label, const Output *o)
fprintf(stderr, "\n");
}
+static int test_xiph_dimensions(void)
+{
+ static const char *const lines[] = {
+ "fmtp:98 width=320; height=240",
+ "fmtp:98 width=1048561; height=1",
+ "fmtp:98 width=1; height=1048561",
+ "fmtp:98 width=321; height=241",
+ "fmtp:98 width=320; height=0",
+ "fmtp:98 width=320; height=1048562",
+ "fmtp:98 width=320; height=2147483648",
+ "fmtp:98 width=320; height=99999999999999999999",
+ "fmtp:98 width=abc; height=240",
+ };
+ const RTPDynamicProtocolHandler *handler = &ff_theora_dynamic_handler;
+ AVFormatContext *fmt = avformat_alloc_context();
+ AVStream *st;
+ PayloadContext *data;
+
+ if (!fmt)
+ return AVERROR(ENOMEM);
+ st = avformat_new_stream(fmt, NULL);
+ data = av_mallocz(handler->priv_data_size);
+ if (!st || !data) {
+ av_free(data);
+ avformat_free_context(fmt);
+ return AVERROR(ENOMEM);
+ }
+
+ /* Exercise the SDP callback without opening a decoder or fabricating
+ * codec headers. Print the resulting state for comparison by FATE. */
+ for (int i = 0; i < FF_ARRAY_ELEMS(lines); i++) {
+ int ret;
+
+ st->codecpar->width = 640;
+ st->codecpar->height = 480;
+ ret = handler->parse_sdp_a_line(fmt, st->index, data, lines[i]);
+ printf("%s: ret=%d width=%d height=%d\n", lines[i], ret,
+ st->codecpar->width, st->codecpar->height);
+ }
+
+ handler->close(data);
+ av_free(data);
+ avformat_free_context(fmt);
+ return 0;
+}
+
int main(void)
{
static uint8_t ts_stream[NB_TS_PACKETS * TS_SIZE];
@@ -260,6 +306,9 @@ int main(void)
static const int reordered[NB_RTP_PACKETS] = { 0, 2, 1, 3, 4 };
static Output ref, got;
+ if (test_xiph_dimensions() < 0)
+ return 2;
+
build_ts_stream(ts_stream);
if (run(ts_stream, in_order, &ref) < 0 ||
diff --git a/tests/Makefile b/tests/Makefile
index 7d1597bdf76..6d823720ae3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -257,6 +257,7 @@ include $(SRC_PATH)/tests/fate/qoa.mak
include $(SRC_PATH)/tests/fate/qt.mak
include $(SRC_PATH)/tests/fate/qtrle.mak
include $(SRC_PATH)/tests/fate/real.mak
+include $(SRC_PATH)/tests/fate/rtpdec-sdp.mak
include $(SRC_PATH)/tests/fate/rtsp.mak
include $(SRC_PATH)/tests/fate/sbc.mak
include $(SRC_PATH)/tests/fate/screen.mak
diff --git a/tests/fate/rtpdec-sdp.mak b/tests/fate/rtpdec-sdp.mak
new file mode 100644
index 00000000000..f1c8ca53da6
--- /dev/null
+++ b/tests/fate/rtpdec-sdp.mak
@@ -0,0 +1,115 @@
+FATE_RTPDEC_SDP_FFPROBE-$(call ALLYES, SDP_DEMUXER FILE_PROTOCOL) \
+ += fate-rtpdec-sdp-h264-framesize \
+ fate-rtpdec-sdp-h264-framesize-zero \
+ fate-rtpdec-sdp-h264-framesize-negative \
+ fate-rtpdec-sdp-h264-framesize-above-int-max
\
+ fate-rtpdec-sdp-h264-framesize-overflow \
+ fate-rtpdec-sdp-h264-framesize-too-large \
+ fate-rtpdec-sdp-h264-framesize-bad-height \
+ fate-rtpdec-sdp-xiph-width-above-max \
+ fate-rtpdec-sdp-xiph-width-negative \
+ fate-rtpdec-sdp-xiph-width-above-int-max \
+ fate-rtpdec-sdp-xiph-width-overflow
+
+# The depacketizers take stream dimensions from the SDP description, so from
+# the server. The descriptions below are the smallest ones that reach the
+# parsers; custom_io keeps the demuxer from opening a transport, which keeps
+# these tests offline.
+RTPDEC_SDP_HEAD = v=0\no=- 0 0 IN IP4 127.0.0.1\ns=rtpdec\nc=IN IP4
127.0.0.1\nt=0 0\n
+
+# a=framesize:<pt> <width>-<height>, handled by ff_h264_parse_framesize() for
+# both H264 and HEVC streams.
+RTPDEC_H264_HEAD = $(RTPDEC_SDP_HEAD)m=video 5004 RTP/AVP 96\na=rtpmap:96
H264/90000\n
+# Warning level, because for the rejected sizes the probed dimensions are the
+# same before and after the size is checked here: av_image_check_size() in the
+# decoder already refused them and left the stream at 0x0. What the check adds
+# is that the value never reaches codecpar, and the diagnostic is what shows
+# which one refused it.
+RTPDEC_PROBE_WARN = run ffprobe$(PROGSSUF)$(EXESUF) -v warning \
+ -sdp_flags custom_io -protocol_whitelist file \
+ -analyzeduration 0 -probesize 32 \
+ -show_entries stream=width,height -print_format compact
+
+define FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD
+ cleanfiles="$${cleanfiles} $${outfile}.sdp"; \
+ printf "$(RTPDEC_H264_HEAD)a=framesize:96 $(1)\n" > "$${outfile}.sdp"; \
+ $(RTPDEC_PROBE_WARN) "$$(target_path "$${outfile}.sdp")"
+endef
+
+# A valid size must still be applied, otherwise the tests below would pass
+# with the attribute never being parsed.
+fate-rtpdec-sdp-h264-framesize: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,320-240)
+
+# Sizes that must be refused. The last one is above the limit
+# av_image_check_size() enforces, the two before it do not fit an int at all,
+# which is where atoi() used to be undefined.
+fate-rtpdec-sdp-h264-framesize-zero: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,0-0)
+fate-rtpdec-sdp-h264-framesize-zero: CMP = grep
+fate-rtpdec-sdp-h264-framesize-zero: REF = Invalid framesize 0-0 in SDP
+
+fate-rtpdec-sdp-h264-framesize-negative: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,-1--1)
+fate-rtpdec-sdp-h264-framesize-negative: CMP = grep
+fate-rtpdec-sdp-h264-framesize-negative: REF = Invalid framesize -1--1 in SDP
+
+fate-rtpdec-sdp-h264-framesize-above-int-max: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,2147483648-2147483648)
+fate-rtpdec-sdp-h264-framesize-above-int-max: CMP = grep
+fate-rtpdec-sdp-h264-framesize-above-int-max: REF = Invalid framesize
2147483648-2147483648 in SDP
+
+fate-rtpdec-sdp-h264-framesize-overflow: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,99999999999999999999-5)
+fate-rtpdec-sdp-h264-framesize-overflow: CMP = grep
+fate-rtpdec-sdp-h264-framesize-overflow: REF = Invalid framesize
99999999999999999999-5 in SDP
+
+fate-rtpdec-sdp-h264-framesize-too-large: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,65536-65536)
+fate-rtpdec-sdp-h264-framesize-too-large: CMP = grep
+fate-rtpdec-sdp-h264-framesize-too-large: REF = Invalid framesize 65536-65536
in SDP
+
+# The only case where it is the height that is refused rather than the width.
+# This is coverage of that side of the parse, not a discriminating control:
+# av_image_check_size() refuses the value atoi() would have produced as well,
+# so the diagnostic appears either way.
+fate-rtpdec-sdp-h264-framesize-bad-height: CMD = $(call
FATE_RTPDEC_SDP_H264_FRAMESIZE_CMD,320-2147483648)
+fate-rtpdec-sdp-h264-framesize-bad-height: CMP = grep
+fate-rtpdec-sdp-h264-framesize-bad-height: REF = Invalid framesize
320-2147483648 in SDP
+
+# The Xiph fmtp width and height, limited to 1 - 1048561 by section 6 of
+# RTP Payload Format for Theora Encoded Video
+# (draft-barbato-avt-rtp-theora-01); RFC 5215 is the Vorbis audio format and
+# says nothing about dimensions.
+# These compare the diagnostic, not the probed size, and discard the exit
+# status: a Xiph stream whose packed header is not a real one cannot be probed
+# at all where the theora decoder is built in, so both the probe output and
+# the exit status depend on the configuration while the diagnostic does not.
+# The accepted-value side of this parser is covered by fate-rtpdec, which
+# calls the SDP callback directly and so needs no decoder at all; the h264
+# positive control below exercises a different assignment path and does not
+# stand in for it.
+RTPDEC_XIPH_HEAD = $(RTPDEC_SDP_HEAD)m=video 5004 RTP/AVP 98\na=rtpmap:98
theora/90000\n
+
+define FATE_RTPDEC_SDP_XIPH_WIDTH_CMD
+ cleanfiles="$${cleanfiles} $${outfile}.sdp"; \
+ printf "$(RTPDEC_XIPH_HEAD)a=fmtp:98 sampling=YCbCr-4:2:0; width=$(1);
height=240; delivery-method=inline; configuration=AA\n" \
+ > "$${outfile}.sdp"; \
+ $(RTPDEC_PROBE_WARN) "$$(target_path "$${outfile}.sdp")" ; true
+endef
+
+# The first value above the limit that draft states, and the three that are
+# outside the range in the other direction or do not fit an int at all.
+fate-rtpdec-sdp-xiph-width-above-max: CMD = $(call
FATE_RTPDEC_SDP_XIPH_WIDTH_CMD,1048562)
+fate-rtpdec-sdp-xiph-width-above-max: CMP = grep
+fate-rtpdec-sdp-xiph-width-above-max: REF = Invalid width 1048562 in SDP
+
+fate-rtpdec-sdp-xiph-width-negative: CMD = $(call
FATE_RTPDEC_SDP_XIPH_WIDTH_CMD,-1)
+fate-rtpdec-sdp-xiph-width-negative: CMP = grep
+fate-rtpdec-sdp-xiph-width-negative: REF = Invalid width -1 in SDP
+
+fate-rtpdec-sdp-xiph-width-above-int-max: CMD = $(call
FATE_RTPDEC_SDP_XIPH_WIDTH_CMD,2147483648)
+fate-rtpdec-sdp-xiph-width-above-int-max: CMP = grep
+fate-rtpdec-sdp-xiph-width-above-int-max: REF = Invalid width 2147483648 in SDP
+
+fate-rtpdec-sdp-xiph-width-overflow: CMD = $(call
FATE_RTPDEC_SDP_XIPH_WIDTH_CMD,99999999999999999999)
+fate-rtpdec-sdp-xiph-width-overflow: CMP = grep
+fate-rtpdec-sdp-xiph-width-overflow: REF = Invalid width 99999999999999999999
in SDP
+
+FATE_FFPROBE += $(FATE_RTPDEC_SDP_FFPROBE-yes)
+
+fate-rtpdec-sdp: $(FATE_RTPDEC_SDP_FFPROBE-yes)
diff --git a/tests/ref/fate/rtpdec b/tests/ref/fate/rtpdec
index 9353f6fb2bd..18508d399c5 100644
--- a/tests/ref/fate/rtpdec
+++ b/tests/ref/fate/rtpdec
@@ -1 +1,10 @@
+fmtp:98 width=320; height=240: ret=0 width=320 height=240
+fmtp:98 width=1048561; height=1: ret=0 width=1048561 height=1
+fmtp:98 width=1; height=1048561: ret=0 width=1 height=1048561
+fmtp:98 width=321; height=241: ret=0 width=321 height=241
+fmtp:98 width=320; height=0: ret=-1094995529 width=320 height=480
+fmtp:98 width=320; height=1048562: ret=-1094995529 width=320 height=480
+fmtp:98 width=320; height=2147483648: ret=-1094995529 width=320 height=480
+fmtp:98 width=320; height=99999999999999999999: ret=-1094995529 width=320
height=480
+fmtp:98 width=abc; height=240: ret=-1094995529 width=640 height=480
OK: 2 PES packets (6238 bytes) recovered identically with and without
reordering
diff --git a/tests/ref/fate/rtpdec-sdp-h264-framesize
b/tests/ref/fate/rtpdec-sdp-h264-framesize
new file mode 100644
index 00000000000..9dca8cc0629
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-h264-framesize
@@ -0,0 +1 @@
+stream|width=320|height=240
--
2.54.0
From 211e4ec597ea722a4e04392c6f8936eb4bdc1646 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 16:55:37 +0300
Subject: [PATCH 5/7] tools/patcheck: Warn about atoi()
atoi() has undefined behaviour when the value does not fit into an int,
so a patch that parses a number from a file or from the network with it
has no way to reject one that does not. The two preceding commits fixed
the last two places in the RTP depacketizers where that reached codecpar;
this is what keeps new ones from arriving unnoticed.
Written in the same form as the existing sprintf/strcat/strcpy hint, and
like it, it is a hint: patcheck says so itself, a muxer parsing its own
option string is usually fine, and being a grep it also matches comments
that merely mention atoi().
Tested on a diff that adds an atoi() call, which it reports, and on one
that does not, which it leaves alone.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
tools/patcheck | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/patcheck b/tools/patcheck
index ab6f73a7846..9f7fa7bb918 100755
--- a/tools/patcheck
+++ b/tools/patcheck
@@ -75,6 +75,7 @@ hiegrep '\) *av_malloc' 'useless casts' $*
hiegrep ':\+ *'"$ERE_PRITYP"' *inline' 'non static inline or strangely ordered
inline+static' $*
hiegrep "$ERE_FUNCS"' *\)' 'missing void' $*
hiegrep '(sprintf|strcat|strcpy)' 'Possible security issue, make sure this is
safe or use snprintf/av_strl*' $*
+hiegrep '\batoi *\(' 'atoi() has undefined behaviour when the value does not
fit an int, use ff_parse_bounded_int() or strtol() with a range check' $*
hiegrep '/
*(2|4|8|16|32|64|128|256|512|1024|2048|4096|8192|16384|32768|65536)[^0-9]'
'divide by 2^x could use >> maybe' $*
hiegrep '#(el|)if *(0|1)' 'useless #if' $*
hiegrep 'if *\( *(0|1) *\)' 'useless if()' $*
--
2.54.0
From ea27a414f5ca40f39c84eb943a5c55e4f4cdd7ca Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Mon, 26 Oct 2026 15:23:22 +0300
Subject: [PATCH 6/7] avformat/rtpdec: Avoid atoi() in the SDP parsers
atoi() has undefined behaviour when the value does not fit into an int, and
where it is implemented as (int)strtol() the value wraps instead. Several of
these parsers range check the attribute *after* converting it, so the check
only ever sees what the conversion produced, and a value far outside the
accepted range can arrive inside it.
That is reachable from the SDP description, i.e. from the server. Measured on
current master, with a minimal SDP fed to the sdp demuxer:
a=fmtp:112 sampling=YCbCr-4:2:2; width=320; height=240; depth=8
-> codec_name=rawvideo width=320 (accepted, as intended)
a=fmtp:112 ... depth=9
-> codec_name=unknown (refused, as intended)
a=fmtp:112 ... depth=4294967304
-> codec_name=rawvideo width=320 (refused nowhere)
4294967304 is neither 8 nor 10, so rfc4175_parse_format() should refuse it. It
arrives there as 8, and the stream is configured as 8 bit UYVY422 from a depth
the sender never asked for. The same wrapping reaches width as
width=4294967616 -> 320, and sprop-maxcapturerate=4294975296 -> 8000, which
the 8000..48000 test in rtpdec_opus.c then accepts without a word while 7999
is refused.
Convert every atoi() in the RTP depacketizers' SDP parsers to
ff_parse_bounded_int(), which refuses what does not fit the range instead of
wrapping into it, and state the range each attribute allows:
rtpdec_rfc4175: width, height 1..INT_MAX
depth 1..INT_MAX, then the existing per-sampling
test, which now sees the real value
rtpdec_opus: sprop-maxcapturerate 8000..48000, the test it already had
rtpdec_ilbc: mode 0..INT_MAX, then the existing switch
rtpdec_amr: octet-align, crc 0..1
interleaving 0..INT_MAX
channels 1..6
rtpdec_h264: packetization-mode 0..2 (RFC 6184)
rtpdec_hevc: profile-id 0..31
sprop-max-don-diff, sprop-depack-buf-nalus 0..32767
rtpdec_av1: profile 0..2, level-idx 0..31, tier 0..1
rdt: StartTime INT_MIN..INT_MAX
The AV1 fields are uint8_t, so atoi() there truncated twice, once into an int
and once into the field. rdt's first_dts is int64_t but atoi() never produced
more than an int, so this keeps the range it could represent rather than
widening it.
Measured after this patch: depth=4294967304, depth=4294967306,
width=4294967616 and sprop-maxcapturerate=4294975296 are all refused, while
depth=8, depth=10, width=320 and sprop-maxcapturerate=48000 are still
accepted, and the 23 existing RTP/SDP FATE tests pass unchanged.
One unrelated thing fixed in passing because it sits on a touched line: the
AV1 "level-idx" diagnostic printed profile rather than level_idx.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
libavformat/rdt.c | 16 ++++++++++++++--
libavformat/rtpdec_amr.c | 22 ++++++++++++++++++----
libavformat/rtpdec_av1.c | 30 ++++++++++++++++++++++++++----
libavformat/rtpdec_h264.c | 11 +++++++++--
libavformat/rtpdec_hevc.c | 23 ++++++++++++++++++++---
libavformat/rtpdec_ilbc.c | 12 +++++++++++-
libavformat/rtpdec_latm.c | 7 ++++++-
libavformat/rtpdec_opus.c | 7 +++++--
libavformat/rtpdec_rfc4175.c | 24 +++++++++++++++++-------
9 files changed, 126 insertions(+), 26 deletions(-)
diff --git a/libavformat/rdt.c b/libavformat/rdt.c
index 8228016aa1b..3e2f06f193a 100644
--- a/libavformat/rdt.c
+++ b/libavformat/rdt.c
@@ -25,6 +25,8 @@
* @author Ronald S. Bultje <[email protected]>
*/
+#include <limits.h>
+
#include "avformat.h"
#include "libavutil/avstring.h"
#include "libavutil/mem.h"
@@ -423,8 +425,18 @@ rdt_parse_sdp_line (AVFormatContext *s, int st_index,
if (av_strstart(p, "OpaqueData:buffer;", &p)) {
rdt->mlti_data = rdt_parse_b64buf(&rdt->mlti_data_size, p);
- } else if (av_strstart(p, "StartTime:integer;", &p))
- ffstream(stream)->first_dts = atoi(p);
+ } else if (av_strstart(p, "StartTime:integer;", &p)) {
+ int start_time;
+
+ /* first_dts is int64_t, but atoi() never produced more than an int
+ * and was undefined above that; keep the range it could represent
+ * and reject the rest. */
+ if (ff_parse_bounded_int(p, INT_MIN, INT_MAX, &start_time) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid StartTime %s in SDP\n", p);
+ return AVERROR_INVALIDDATA;
+ }
+ ffstream(stream)->first_dts = start_time;
+ }
else if (av_strstart(p, "ASMRuleBook:string;", &p)) {
int n, first = -1;
diff --git a/libavformat/rtpdec_amr.c b/libavformat/rtpdec_amr.c
index ca4f0ff03ac..77b7fd2bf16 100644
--- a/libavformat/rtpdec_amr.c
+++ b/libavformat/rtpdec_amr.c
@@ -20,7 +20,10 @@
*/
#include "libavutil/channel_layout.h"
+#include <limits.h>
+
#include "avformat.h"
+#include "internal.h"
#include "rtpdec_formats.h"
#include "libavutil/avstring.h"
@@ -136,6 +139,8 @@ static int amr_parse_fmtp(AVFormatContext *s,
AVStream *stream, PayloadContext *data,
const char *attr, const char *value)
{
+ int ret = 0;
+
/* Some AMR SDP configurations contain "octet-align", without
* the trailing =1. Therefore, if the value is empty,
* interpret it as "1".
@@ -145,14 +150,23 @@ static int amr_parse_fmtp(AVFormatContext *s,
"nonstandard empty value\n", attr);
value = "1";
}
+ /* amr_parse_sdp_line() below requires octet-align=1, crc=0,
+ * interleaving=0 and channels=1. That check only sees what the conversion
+ * produced, so the conversion must not wrap a larger value into the
+ * accepted one. */
if (!strcmp(attr, "octet-align"))
- data->octet_align = atoi(value);
+ ret = ff_parse_bounded_int(value, 0, 1, &data->octet_align);
else if (!strcmp(attr, "crc"))
- data->crc = atoi(value);
+ ret = ff_parse_bounded_int(value, 0, 1, &data->crc);
else if (!strcmp(attr, "interleaving"))
- data->interleaving = atoi(value);
+ ret = ff_parse_bounded_int(value, 0, INT_MAX, &data->interleaving);
else if (!strcmp(attr, "channels"))
- data->channels = atoi(value);
+ ret = ff_parse_bounded_int(value, 1, 6, &data->channels);
+ if (ret < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid AMR %s value %s in SDP\n", attr,
+ value);
+ return AVERROR_INVALIDDATA;
+ }
return 0;
}
diff --git a/libavformat/rtpdec_av1.c b/libavformat/rtpdec_av1.c
index a45a2f149d6..6b91fa158df 100644
--- a/libavformat/rtpdec_av1.c
+++ b/libavformat/rtpdec_av1.c
@@ -32,6 +32,7 @@
#include "libavutil/avstring.h"
#include "libavutil/mem.h"
#include "avformat.h"
+#include "internal.h"
#include "rtpdec.h"
#include "libavcodec/av1.h"
@@ -63,14 +64,35 @@ static int sdp_parse_fmtp_config_av1(AVFormatContext *s,
AVStream *stream,
PayloadContext *av1_data,
const char *attr, const char *value) {
+ /* The fields below are uint8_t, so atoi() used to truncate twice: once
+ * into an int and once into the field. The ranges are those of the AV1
+ * RTP payload format. */
if (!strcmp(attr, "profile")) {
- av1_data->profile = atoi(value);
+ int profile;
+
+ if (ff_parse_bounded_int(value, 0, 2, &profile) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid AV1 profile %s in SDP\n", value);
+ return AVERROR_INVALIDDATA;
+ }
+ av1_data->profile = profile;
av_log(s, AV_LOG_DEBUG, "RTP AV1 profile: %u\n", av1_data->profile);
} else if (!strcmp(attr, "level-idx")) {
- av1_data->level_idx = atoi(value);
- av_log(s, AV_LOG_DEBUG, "RTP AV1 level: %u\n", av1_data->profile);
+ int level_idx;
+
+ if (ff_parse_bounded_int(value, 0, 31, &level_idx) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid AV1 level-idx %s in SDP\n",
value);
+ return AVERROR_INVALIDDATA;
+ }
+ av1_data->level_idx = level_idx;
+ av_log(s, AV_LOG_DEBUG, "RTP AV1 level: %u\n", av1_data->level_idx);
} else if (!strcmp(attr, "tier")) {
- av1_data->tier = atoi(value);
+ int tier;
+
+ if (ff_parse_bounded_int(value, 0, 1, &tier) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid AV1 tier %s in SDP\n", value);
+ return AVERROR_INVALIDDATA;
+ }
+ av1_data->tier = tier;
av_log(s, AV_LOG_DEBUG, "RTP AV1 tier: %u\n", av1_data->tier);
}
return 0;
diff --git a/libavformat/rtpdec_h264.c b/libavformat/rtpdec_h264.c
index c749fab5590..7ece3c1df7f 100644
--- a/libavformat/rtpdec_h264.c
+++ b/libavformat/rtpdec_h264.c
@@ -154,8 +154,15 @@ static int sdp_parse_fmtp_config_h264(AVFormatContext *s,
AVCodecParameters *par = stream->codecpar;
if (!strcmp(attr, "packetization-mode")) {
- av_log(s, AV_LOG_DEBUG, "RTP Packetization Mode: %d\n", atoi(value));
- h264_data->packetization_mode = atoi(value);
+ /* RFC 6184 defines modes 0 to 2. */
+ if (ff_parse_bounded_int(value, 0, 2,
+ &h264_data->packetization_mode) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid packetization mode %s in SDP\n",
+ value);
+ return AVERROR_INVALIDDATA;
+ }
+ av_log(s, AV_LOG_DEBUG, "RTP Packetization Mode: %d\n",
+ h264_data->packetization_mode);
/*
* Packetization Mode:
* 0 or not present: Single NAL mode (Only nals from 1-23 are allowed)
diff --git a/libavformat/rtpdec_hevc.c b/libavformat/rtpdec_hevc.c
index 47da291db27..ec241191805 100644
--- a/libavformat/rtpdec_hevc.c
+++ b/libavformat/rtpdec_hevc.c
@@ -53,7 +53,10 @@ static av_cold int
hevc_sdp_parse_fmtp_config(AVFormatContext *s,
/* profile-space: 0-3 */
/* profile-id: 0-31 */
if (!strcmp(attr, "profile-id")) {
- hevc_data->profile_id = atoi(value);
+ if (ff_parse_bounded_int(value, 0, 31, &hevc_data->profile_id) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid profile-id %s in SDP\n", value);
+ return AVERROR_INVALIDDATA;
+ }
av_log(s, AV_LOG_TRACE, "SDP: found profile-id: %d\n",
hevc_data->profile_id);
}
@@ -103,7 +106,14 @@ static av_cold int
hevc_sdp_parse_fmtp_config(AVFormatContext *s,
value MUST be greater than 0.
*/
if (!strcmp(attr, "sprop-max-don-diff")) {
- if (atoi(value) > 0)
+ int max_don_diff;
+
+ if (ff_parse_bounded_int(value, 0, 32767, &max_don_diff) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid sprop-max-don-diff %s in SDP\n",
+ value);
+ return AVERROR_INVALIDDATA;
+ }
+ if (max_don_diff > 0)
hevc_data->using_donl_field = 1;
av_log(s, AV_LOG_TRACE, "Found sprop-max-don-diff in SDP, DON field
usage is: %d\n",
hevc_data->using_donl_field);
@@ -111,7 +121,14 @@ static av_cold int
hevc_sdp_parse_fmtp_config(AVFormatContext *s,
/* sprop-depack-buf-nalus: 0-32767 */
if (!strcmp(attr, "sprop-depack-buf-nalus")) {
- if (atoi(value) > 0)
+ int depack_buf_nalus;
+
+ if (ff_parse_bounded_int(value, 0, 32767, &depack_buf_nalus) < 0) {
+ av_log(s, AV_LOG_ERROR,
+ "Invalid sprop-depack-buf-nalus %s in SDP\n", value);
+ return AVERROR_INVALIDDATA;
+ }
+ if (depack_buf_nalus > 0)
hevc_data->using_donl_field = 1;
av_log(s, AV_LOG_TRACE, "Found sprop-depack-buf-nalus in SDP, DON
field usage is: %d\n",
hevc_data->using_donl_field);
diff --git a/libavformat/rtpdec_ilbc.c b/libavformat/rtpdec_ilbc.c
index 9094f2c786f..b608a181b25 100644
--- a/libavformat/rtpdec_ilbc.c
+++ b/libavformat/rtpdec_ilbc.c
@@ -19,7 +19,10 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <limits.h>
+
#include "avformat.h"
+#include "internal.h"
#include "rtpdec_formats.h"
#include "libavutil/avstring.h"
@@ -28,7 +31,14 @@ static int ilbc_parse_fmtp(AVFormatContext *s,
const char *attr, const char *value)
{
if (!strcmp(attr, "mode")) {
- int mode = atoi(value);
+ int mode;
+
+ /* The switch below only sees what the conversion produced, so the
+ * conversion must not wrap a larger value into 20 or 30. */
+ if (ff_parse_bounded_int(value, 0, INT_MAX, &mode) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid iLBC mode %s in SDP\n", value);
+ return AVERROR(EINVAL);
+ }
switch (mode) {
case 20:
stream->codecpar->block_align = 38;
diff --git a/libavformat/rtpdec_latm.c b/libavformat/rtpdec_latm.c
index dd374a0e6ae..b04c5d8e10b 100644
--- a/libavformat/rtpdec_latm.c
+++ b/libavformat/rtpdec_latm.c
@@ -145,7 +145,12 @@ static int parse_fmtp(AVFormatContext *s,
if (res < 0)
return res;
} else if (!strcmp(attr, "cpresent")) {
- int cpresent = atoi(value);
+ int cpresent;
+
+ if (ff_parse_bounded_int(value, 0, 1, &cpresent) < 0) {
+ av_log(s, AV_LOG_ERROR, "Invalid cpresent %s in SDP\n", value);
+ return AVERROR_INVALIDDATA;
+ }
if (cpresent != 0)
avpriv_request_sample(s,
"RTP MP4A-LATM with in-band configuration");
diff --git a/libavformat/rtpdec_opus.c b/libavformat/rtpdec_opus.c
index 1eeef3c4b6a..112b2c8479d 100644
--- a/libavformat/rtpdec_opus.c
+++ b/libavformat/rtpdec_opus.c
@@ -118,8 +118,11 @@ static int parse_fmtp(AVFormatContext *s,
const char *attr, const char *value)
{
if (!strcmp(attr, "sprop-maxcapturerate")) {
- int rate = atoi(value);
- if (rate < 8000 || rate > 48000) {
+ int rate;
+
+ /* The range test below only sees what the conversion produced, so the
+ * conversion must not wrap a larger value into the accepted range. */
+ if (ff_parse_bounded_int(value, 8000, 48000, &rate) < 0) {
av_log(s, AV_LOG_ERROR,
"fmtp field 'sprop-maxcapturerate' must be between 8000 to
48000 (provided value: %s)",
value);
diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
index a73f59e9847..0dd1876e960 100644
--- a/libavformat/rtpdec_rfc4175.c
+++ b/libavformat/rtpdec_rfc4175.c
@@ -21,7 +21,10 @@
/* Development sponsored by CBC/Radio-Canada */
+#include <limits.h>
+
#include "avio_internal.h"
+#include "internal.h"
#include "rtpdec_formats.h"
#include "libavutil/avassert.h"
#include "libavutil/avstring.h"
@@ -126,14 +129,21 @@ static int rfc4175_parse_fmtp(AVFormatContext *s,
AVStream *stream,
PayloadContext *data, const char *attr,
const char *value)
{
- if (!strncmp(attr, "width", 5))
- data->width = atoi(value);
- else if (!strncmp(attr, "height", 6))
- data->height = atoi(value);
- else if (data->sampling == NULL && !strncmp(attr, "sampling", 8))
+ if (!strncmp(attr, "width", 5)) {
+ if (ff_parse_bounded_int(value, 1, INT_MAX, &data->width) < 0)
+ return AVERROR_INVALIDDATA;
+ } else if (!strncmp(attr, "height", 6)) {
+ if (ff_parse_bounded_int(value, 1, INT_MAX, &data->height) < 0)
+ return AVERROR_INVALIDDATA;
+ } else if (data->sampling == NULL && !strncmp(attr, "sampling", 8))
data->sampling = av_strdup(value);
- else if (!strncmp(attr, "depth", 5))
- data->depth = atoi(value);
+ else if (!strncmp(attr, "depth", 5)) {
+ /* The depth values this parser accepts are checked against the
+ * sampling in rfc4175_parse_format(); that check only sees what the
+ * conversion produced, so the conversion must not wrap. */
+ if (ff_parse_bounded_int(value, 1, INT_MAX, &data->depth) < 0)
+ return AVERROR_INVALIDDATA;
+ }
else if (!strncmp(attr, "interlace", 9))
data->interlaced = 1;
else if (!strncmp(attr, "exactframerate", 14)) {
--
2.54.0
From 1747a1819828c02b6923d6351150c1b4404307ab Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Mon, 26 Oct 2026 15:23:36 +0300
Subject: [PATCH 7/7] tests/fate/rtpdec-sdp: Test the wrapped SDP attribute
values
Seven tests for the previous commit, none of which needs an external sample or
a network connection.
Measured which of them depend on it, by reverting only that commit's nine
source files while keeping the tests:
| test | without the commit |
| --- | --- |
| fate-rtpdec-sdp-rfc4175-depth | passes |
| fate-rtpdec-sdp-rfc4175-depth-10 | passes |
| fate-rtpdec-sdp-rfc4175-depth-invalid | passes |
| fate-rtpdec-sdp-rfc4175-depth-wrapped | fails |
| fate-rtpdec-sdp-rfc4175-width-wrapped | fails |
| fate-rtpdec-sdp-opus-rate | passes |
| fate-rtpdec-sdp-opus-rate-wrapped | fails |
The three failures are the regression coverage: each sends a value that wraps
into the accepted range once truncated into an int, which is what the previous
commit stops happening. The four that pass either way are there on purpose:
* -rfc4175-depth and -rfc4175-depth-10 are the two accepted depths and
-opus-rate an accepted rate, so a change that refused them would be caught.
Without them the three wrapped tests could pass while nothing was accepted
at all.
* -rfc4175-depth-invalid sends 9, which is outside the accepted set but inside
the int range, so it is refused before and after. It pairs with
-rfc4175-depth-wrapped: the same outcome has to be reached whether the value
is out of range directly or only after wrapping.
-rfc4175-depth-invalid still reports width=320 while -rfc4175-depth-wrapped
reports width=0, because a depth that cannot be converted ends the fmtp line
before the width is applied, whereas 9 converts and is refused later in
rfc4175_parse_format(). Both references record what each path actually does.
The opus tests compare different things for the same reason as the Xiph ones:
the reported rate is the one the rtpmap line established either way, so the
refusal is only visible in the diagnostic.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
tests/fate/rtpdec-sdp.mak | 72 ++++++++++++++++++-
tests/ref/fate/rtpdec-sdp-opus-rate | 1 +
tests/ref/fate/rtpdec-sdp-rfc4175-depth | 1 +
tests/ref/fate/rtpdec-sdp-rfc4175-depth-10 | 1 +
.../ref/fate/rtpdec-sdp-rfc4175-depth-invalid | 1 +
.../ref/fate/rtpdec-sdp-rfc4175-depth-wrapped | 1 +
.../ref/fate/rtpdec-sdp-rfc4175-width-wrapped | 1 +
7 files changed, 77 insertions(+), 1 deletion(-)
create mode 100644 tests/ref/fate/rtpdec-sdp-opus-rate
create mode 100644 tests/ref/fate/rtpdec-sdp-rfc4175-depth
create mode 100644 tests/ref/fate/rtpdec-sdp-rfc4175-depth-10
create mode 100644 tests/ref/fate/rtpdec-sdp-rfc4175-depth-invalid
create mode 100644 tests/ref/fate/rtpdec-sdp-rfc4175-depth-wrapped
create mode 100644 tests/ref/fate/rtpdec-sdp-rfc4175-width-wrapped
diff --git a/tests/fate/rtpdec-sdp.mak b/tests/fate/rtpdec-sdp.mak
index f1c8ca53da6..0b769b79f05 100644
--- a/tests/fate/rtpdec-sdp.mak
+++ b/tests/fate/rtpdec-sdp.mak
@@ -9,7 +9,14 @@ FATE_RTPDEC_SDP_FFPROBE-$(call ALLYES, SDP_DEMUXER
FILE_PROTOCOL) \
fate-rtpdec-sdp-xiph-width-above-max \
fate-rtpdec-sdp-xiph-width-negative \
fate-rtpdec-sdp-xiph-width-above-int-max \
- fate-rtpdec-sdp-xiph-width-overflow
+ fate-rtpdec-sdp-xiph-width-overflow \
+ fate-rtpdec-sdp-rfc4175-depth \
+ fate-rtpdec-sdp-rfc4175-depth-10 \
+ fate-rtpdec-sdp-rfc4175-depth-invalid \
+ fate-rtpdec-sdp-rfc4175-depth-wrapped \
+ fate-rtpdec-sdp-rfc4175-width-wrapped \
+ fate-rtpdec-sdp-opus-rate \
+ fate-rtpdec-sdp-opus-rate-wrapped
# The depacketizers take stream dimensions from the SDP description, so from
# the server. The descriptions below are the smallest ones that reach the
@@ -113,3 +120,66 @@ fate-rtpdec-sdp-xiph-width-overflow: REF = Invalid width
99999999999999999999 in
FATE_FFPROBE += $(FATE_RTPDEC_SDP_FFPROBE-yes)
fate-rtpdec-sdp: $(FATE_RTPDEC_SDP_FFPROBE-yes)
+
+# The RFC 4175 and Opus fmtp attributes check their value after converting it.
+# atoi() truncates into an int, so a value that is far outside the accepted
+# range can arrive inside it: depth=4294967304 reached the "depth == 8" test as
+# 8 and a stream was configured from a depth the sender never asked for. These
+# read the resulting codec back, which is where that was visible.
+RTPDEC_RFC4175_HEAD = $(RTPDEC_SDP_HEAD)m=video 5004 RTP/AVP 112\na=rtpmap:112
raw/90000\n
+RTPDEC_CODEC_PROBE = run ffprobe$(PROGSSUF)$(EXESUF) -v error \
+ -sdp_flags custom_io -protocol_whitelist file \
+ -analyzeduration 0 -probesize 32 \
+ -show_entries stream=codec_name,width -print_format compact
+
+define FATE_RTPDEC_SDP_RFC4175_CMD
+ cleanfiles="$${cleanfiles} $${outfile}.sdp"; \
+ printf "$(RTPDEC_RFC4175_HEAD)a=fmtp:112 sampling=YCbCr-4:2:2; width=$(1);
height=240; depth=$(2)\n" \
+ > "$${outfile}.sdp"; \
+ $(RTPDEC_CODEC_PROBE) "$$(target_path "$${outfile}.sdp")"
+endef
+
+# Both accepted depths, so a change that refused them would be caught.
+fate-rtpdec-sdp-rfc4175-depth: CMD = $(call FATE_RTPDEC_SDP_RFC4175_CMD,320,8)
+fate-rtpdec-sdp-rfc4175-depth-10: CMD = $(call
FATE_RTPDEC_SDP_RFC4175_CMD,320,10)
+
+# A depth outside the accepted set, refused before and after.
+fate-rtpdec-sdp-rfc4175-depth-invalid: CMD = $(call
FATE_RTPDEC_SDP_RFC4175_CMD,320,9)
+
+# The same, reached by wrapping: 4294967304 arrives as 8 and 4294967616 as 320
+# once truncated into an int.
+fate-rtpdec-sdp-rfc4175-depth-wrapped: CMD = $(call
FATE_RTPDEC_SDP_RFC4175_CMD,320,4294967304)
+fate-rtpdec-sdp-rfc4175-width-wrapped: CMD = $(call
FATE_RTPDEC_SDP_RFC4175_CMD,4294967616,8)
+
+# sprop-maxcapturerate is tested against 8000..48000 after the conversion;
+# 4294975296 arrives as 8000. The diagnostic is what shows it was refused,
+# since the rate the rtpmap line established is reported either way.
+RTPDEC_OPUS_HEAD = $(RTPDEC_SDP_HEAD)m=audio 5004 RTP/AVP 97\na=rtpmap:97
opus/48000/2\n
+RTPDEC_RATE_PROBE = run ffprobe$(PROGSSUF)$(EXESUF) -v error \
+ -sdp_flags custom_io -protocol_whitelist file \
+ -analyzeduration 0 -probesize 32 \
+ -show_entries stream=sample_rate -print_format compact
+
+define FATE_RTPDEC_SDP_OPUS_CMD
+ cleanfiles="$${cleanfiles} $${outfile}.sdp"; \
+ printf "$(RTPDEC_OPUS_HEAD)a=fmtp:97 sprop-maxcapturerate=$(1)\n" \
+ > "$${outfile}.sdp"; \
+ $(RTPDEC_RATE_PROBE) "$$(target_path "$${outfile}.sdp")"
+endef
+
+# An accepted rate, so a change that refused them would be caught.
+fate-rtpdec-sdp-opus-rate: CMD = $(call FATE_RTPDEC_SDP_OPUS_CMD,48000)
+
+# 4294975296 arrives as 8000 once truncated into an int. The reported rate is
+# the one the rtpmap line established either way, so the diagnostic is what
+# shows the attribute was refused.
+define FATE_RTPDEC_SDP_OPUS_WARN_CMD
+ cleanfiles="$${cleanfiles} $${outfile}.sdp"; \
+ printf "$(RTPDEC_OPUS_HEAD)a=fmtp:97 sprop-maxcapturerate=$(1)\n" \
+ > "$${outfile}.sdp"; \
+ $(RTPDEC_PROBE_WARN) "$$(target_path "$${outfile}.sdp")" ; true
+endef
+
+fate-rtpdec-sdp-opus-rate-wrapped: CMD = $(call
FATE_RTPDEC_SDP_OPUS_WARN_CMD,4294975296)
+fate-rtpdec-sdp-opus-rate-wrapped: CMP = grep
+fate-rtpdec-sdp-opus-rate-wrapped: REF = sprop-maxcapturerate
diff --git a/tests/ref/fate/rtpdec-sdp-opus-rate
b/tests/ref/fate/rtpdec-sdp-opus-rate
new file mode 100644
index 00000000000..3060935dcef
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-opus-rate
@@ -0,0 +1 @@
+stream|sample_rate=48000
diff --git a/tests/ref/fate/rtpdec-sdp-rfc4175-depth
b/tests/ref/fate/rtpdec-sdp-rfc4175-depth
new file mode 100644
index 00000000000..1b01d068224
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-rfc4175-depth
@@ -0,0 +1 @@
+stream|codec_name=rawvideo|width=320
diff --git a/tests/ref/fate/rtpdec-sdp-rfc4175-depth-10
b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-10
new file mode 100644
index 00000000000..0467f7ec0cf
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-10
@@ -0,0 +1 @@
+stream|codec_name=bitpacked|width=320
diff --git a/tests/ref/fate/rtpdec-sdp-rfc4175-depth-invalid
b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-invalid
new file mode 100644
index 00000000000..610349645cd
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-invalid
@@ -0,0 +1 @@
+stream|codec_name=unknown|width=320
diff --git a/tests/ref/fate/rtpdec-sdp-rfc4175-depth-wrapped
b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-wrapped
new file mode 100644
index 00000000000..daf40eabbcc
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-rfc4175-depth-wrapped
@@ -0,0 +1 @@
+stream|codec_name=unknown|width=0
diff --git a/tests/ref/fate/rtpdec-sdp-rfc4175-width-wrapped
b/tests/ref/fate/rtpdec-sdp-rfc4175-width-wrapped
new file mode 100644
index 00000000000..daf40eabbcc
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-rfc4175-width-wrapped
@@ -0,0 +1 @@
+stream|codec_name=unknown|width=0
--
2.54.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
