Re: [FFmpeg-devel] [PATCH] avutil/timecode: Check for integer overflow in av_timecode_init_from_components() (PR #20236)

[Kieran Kunhya via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

On Wed, 13 Aug 2025, 14:25 michaelni, <code@ffmpeg.org> wrote:

> PR #20236 opened by michaelni
> URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20236
> Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20236.patch
>
> Fixes: integer overflow
> Fixes: testcase that calls av_timecode_init_from_components() with hh set
> explicitly to INT_MAX
>
> Found-by: Youngjae Choi, Mingyoung Ban, Seunghoon Woo
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>
>
> From 0762e660ff8fb8c2f4c3d46a6a6c821bd69633e6 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michael@niedermayer.cc>
> Date: Thu, 14 Aug 2025 02:12:26 +0200
> Subject: [PATCH] avutil/timecode: Check for integer overflow in
>  av_timecode_init_from_components()
>
> Fixes: integer overflow
> Fixes: testcase that calls av_timecode_init_from_components() with hh set
> explicitly to INT_MAX
>
> Found-by: Youngjae Choi, Mingyoung Ban, Seunghoon Woo
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavutil/timecode.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libavutil/timecode.c b/libavutil/timecode.c
> index bca16b6ac2..052c488071 100644
> --- a/libavutil/timecode.c
> +++ b/libavutil/timecode.c
> @@ -211,6 +211,7 @@ int av_timecode_init(AVTimecode *tc, AVRational rate,
> int flags, int frame_start
>  int av_timecode_init_from_components(AVTimecode *tc, AVRational rate, int
> flags, int hh, int mm, int ss, int ff, void *log_ctx)
>  {
>      int ret;
> +    int64_t s;
>
>      memset(tc, 0, sizeof(*tc));
>      tc->flags = flags;
> @@ -221,7 +222,15 @@ int av_timecode_init_from_components(AVTimecode *tc,
> AVRational rate, int flags,
>      if (ret < 0)
>          return ret;
>
> -    tc->start = (hh*3600 + mm*60 + ss) * tc->fps + ff;
> +    s = hh*3600LL + mm*60LL + ss;
> +    if (s != (int32_t)s)
> +        return AVERROR(EINVAL);
> +
> +    s = s * tc->fps + ff;
> +    if (s != (int32_t)s)
> +        return AVERROR(EINVAL);
> +    tc->start = s;
> +
>      if (tc->flags & AV_TIMECODE_FLAG_DROPFRAME) { /* adjust frame number
> */
>          int tmins = 60*hh + mm;
>          tc->start -= (tc->fps / 30 * 2) * (tmins - tmins/10);
> --
> 2.49.1
>

What is the actual security benefit of this? If someone chooses INT_MAX as
their timecode value, surely they have to expect it overflows?

Kieran

>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".