PR #25181 opened by furkanefecan
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25181
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25181.patch
Five commits closing the last two places in the RTP depacketizers where a
number from the SDP description reached `AVCodecParameters` without being
checked, plus the helper they use and a patcheck hint so new ones do not
arrive unnoticed.
This continues the two sample rate validations merged as `a7b20ee103` and
`3be6c2d943`: same class, same place, found by enumerating it rather than one
at a time.
## What was wrong
`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 comes from a file or from the
network. I read every one of those call sites in the RTP/SDP path. Most are
already guarded, and I am not touching those:
* `rtpdec_rfc4175.c` calls `av_image_check_size()` and requires a depth of
exactly 8 or 10 per sampling.
* `rtpdec_amr.c` rejects anything but `channels=1`, `crc=0`, `interleaving=0`.
* `rtpdec_opus.c` requires 8000..48000, `rtpdec_ilbc.c` a `switch` with a
rejecting `default`.
* `rtsp.c`'s payload types feed `ff_rtp_enc_name()` and
`ff_rtp_get_codec_info()`, which are linear searches over a sentinel
terminated table, so an out of range payload type simply does not match -
I checked this specifically because an array index there would have been a
real out of bounds read.
Two were not guarded:
**`ff_h264_parse_framesize()`**, used for both H264 and HEVC streams:
```c
par->width = atoi(buf1);
par->height = atoi(p + 1); // skip the -
```
**the Xiph `fmtp` width and height**, where the comment states the range
RFC 5215 gives and nothing enforced it:
```c
} else if (!strcmp(attr, "width")) {
/* This is an integer between 1 and 1048561
* and MUST be in multiples of 16. */
par->width = atoi(value);
```
## Measured
Minimal SDP descriptions fed to the sdp demuxer, before the patches:
```
a=framesize:96 2147483648-2147483648 -> 2147483648x2147483648 (INT_MIN,
printed unsigned)
a=framesize:96 99999999999999999999-5 -> 4294967295x5 (-1, printed
unsigned)
a=framesize:96 65536-65536 -> 65536x65536
width=2147483648 -> codecpar width=-2147483648
width=99999999999999999999 -> codecpar width=-1
width=1048562 -> codecpar width=1048562
width=-1 -> codecpar width=-1
```
The framesize values are refused by the decoder's own
`av_image_check_size()`, so for those this is defence in depth: what changes
is that the value no longer reaches codecpar and that the diagnostic names
which check refused it. The Xiph ones are not - nothing downstream cleared
them, so `AVCodecParameters::width` was reported as `INT_MIN` to the caller.
The commit messages state this difference per patch rather than claiming more
than it is.
After the patches every line above is refused with a diagnostic naming the
value, and `320-240`, `width=320`, `width=1048561` and
`a=framesize:96 320-240 trailing` are still accepted unchanged.
## The helper
libavformat has 84 hand-written `strtol()` calls, each with its own idea of
which failures to check: some test `errno` for `ERANGE`, some the range, some
neither. The range check is the easy one to forget, and forgetting it is what
lets the value through. `ff_parse_bounded_int()` makes the caller state the
range it accepts:
```c
if (ff_parse_bounded_int(value, 1, 1048561, &size) < 0)
return AVERROR_INVALIDDATA;
```
`strtoll()` rather than `strtol()` so the `ERANGE` boundary is beyond the int
range everywhere and the range test is what rejects, not the width of `long`.
Base 10 is fixed, because with base 0 a leading zero would select octal and
silently reinterpret decimal values - the same reason the merged `rtsp.c` fix
pins the base.
## Tests
Ten new tests in `tests/fate/rtpdec-sdp.mak`, no external sample and no
network connection: the descriptions are generated inline and
`-sdp_flags custom_io` keeps the demuxer from opening a transport.
Measured which of them depend on the fixes, by restoring libavformat to
`eceaa54a38` while keeping the tests: **nine fail, one passes either way.**
That one is `fate-rtpdec-sdp-h264-framesize`, the positive control, without
which the rest could pass while the attribute was never parsed at all. The
full matrix is in the test commit message.
All ten compare the diagnostic rather than the probed size, for two different
reasons, and I would rather say both than leave one implied:
* For the framesize values the probed size is `0x0` either way - 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. My first version of
these tests compared the probed size and passed in a build configured for
just what they need, then failed all six in a full build, because there
`ffprobe` cannot open the stream and prints nothing. The diagnostic does not
depend on the configuration, so that is what they compare, and they discard
the exit status for the same reason, as `fate-filter-pan-channel-id-limit`
and the `fate-id3v2-*` tests do. That is also why there is no
accepted-value test on the Xiph side; the h264 positive control carries that
part.
Verified in both directions: ten pass and the nine fail as above in a build
configured for only the components these tests need **and** in a full
`--disable-autodetect` build.
The group is named `rtpdec-sdp` because `fate-rtpdec` is already the
`libavformat/tests/rtpdec` unit test; I checked that the existing test still
passes unchanged.
## Notes for review
* Each of the five commits builds and passes on its own; verified by checking
each out and building it in the full configuration.
* `tools/patcheck` is the last commit so it can be dropped on its own if you
would rather not have the hint. Like the existing sprintf/strcat/strcpy hint
it is a grep, so it also fires on comments that merely mention `atoi()`,
including the ones in this series.
* `ff_h264_parse_framesize()` gained a log context parameter so it can say why
it ignored an attribute; both call sites already had one.
* Beyond the makefile matches patcheck always reports for C syntax in
makefiles, it reports nothing new on this series.
Written with AI assistance, noted per-commit in the Co-Authored-By trailers.
Every number above was produced by running the tests.
>From 8272c673f3752413dde0411087e670c62a1841cd 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/5] 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 53dc7b939b..5cc45d71ad 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -423,6 +423,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 4874a825a0..2a7a25a8aa 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 7ee34eb42d0056336d629ac348db216ac0a9da45 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/5] 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 1ff2a72d2a..259001c70b 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 c145edaa5e..c749fab559 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 0e29efc3d6..47da291db2 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 fbefbabf063e191c74af0141c327bf01eb750ab7 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 12:33:47 +0300
Subject: [PATCH 3/5] 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 RFC 5215 gives, 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
multiple of 16 requirement is deliberately not enforced; streams in the
wild do not always honour it, and the range is what keeps the value
representable.
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 | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/libavformat/rtpdec_xiph.c b/libavformat/rtpdec_xiph.c
index 98e4db48f9..756d4b63cf 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")) {
+ } else if (!strcmp(attr, "width") || !strcmp(attr, "height")) {
/* 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);
+ * and MUST be 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 42773c049ba587553b74c6eea3ae431c6f0bbd87 Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 12:44:09 +0300
Subject: [PATCH 4/5] tests/fate/rtpdec-sdp: Test the SDP dimension attributes
Ten 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.
Measured which tests depend on the fixes, by restoring libavformat to
eceaa54a38 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-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 |
Nine failures are the regression coverage. fate-rtpdec-sdp-h264-framesize is
the positive control: without it the others could pass while the attribute was
never parsed at all.
All of them 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 is also why there is no accepted-value test
on the Xiph side, and why the h264 positive control carries that part.
Measured both ways round: all ten pass and the nine fail as above both in a
build configured for only the components these tests need and in a full
--disable-autodetect build.
The group is named rtpdec-sdp because fate-rtpdec is already the
libavformat/tests/rtpdec unit test, which still passes unchanged.
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Furkan Efe Can <[email protected]>
---
tests/Makefile | 1 +
tests/fate/rtpdec-sdp.mak | 102 +++++++++++++++++++++++
tests/ref/fate/rtpdec-sdp-h264-framesize | 1 +
3 files changed, 104 insertions(+)
create mode 100644 tests/fate/rtpdec-sdp.mak
create mode 100644 tests/ref/fate/rtpdec-sdp-h264-framesize
diff --git a/tests/Makefile b/tests/Makefile
index 97e9d39339..bc3e148533 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -244,6 +244,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/sbc.mak
include $(SRC_PATH)/tests/fate/screen.mak
include $(SRC_PATH)/tests/fate/segafilm.mak
diff --git a/tests/fate/rtpdec-sdp.mak b/tests/fate/rtpdec-sdp.mak
new file mode 100644
index 0000000000..850cc11288
--- /dev/null
+++ b/tests/fate/rtpdec-sdp.mak
@@ -0,0 +1,102 @@
+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-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 Xiph fmtp width and height, which RFC 5215 limits to 1 - 1048561.
+# 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.
+# There is no accepted-value test here for the same reason;
+# fate-rtpdec-sdp-h264-framesize is what shows that a valid value is still
+# applied.
+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 RFC 5215 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-sdp-h264-framesize
b/tests/ref/fate/rtpdec-sdp-h264-framesize
new file mode 100644
index 0000000000..9dca8cc062
--- /dev/null
+++ b/tests/ref/fate/rtpdec-sdp-h264-framesize
@@ -0,0 +1 @@
+stream|width=320|height=240
--
2.54.0
>From b824a25b694958a1d51433bfc7a292fe1e4c3aec Mon Sep 17 00:00:00 2001
From: Furkan Efe Can <[email protected]>
Date: Sun, 18 Oct 2026 12:44:09 +0300
Subject: [PATCH 5/5] 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 ab6f73a784..9f7fa7bb91 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
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
