Re: [FFmpeg-devel] [PATCH 5/9] avformat/mov: Check extend and base offset

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 5/9] avformat/mov: Check extend and base offset
Date: Fri, 21 Jun 2024 00:54:55 +0200
Message-ID: <20240620225455.GT4991@pb2> (raw)
In-Reply-To: <E96BDCF6-185B-4208-BD49-3350BCD35DBB@remlab.net>

[-- Attachment #1.1: Type: text/plain, Size: 2407 bytes --]

On Wed, Jun 19, 2024 at 03:08:58PM +0200, Rémi Denis-Courmont wrote:
> 
> 
> Le 19 juin 2024 14:34:59 GMT+02:00, James Almer <jamrial@gmail.com> a écrit :
> >On 6/18/2024 4:07 AM, Rémi Denis-Courmont wrote:
> >> 
> >> 
> >> Le 17 juin 2024 01:08:27 GMT+02:00, Michael Niedermayer <michael@niedermayer.cc> a écrit :
> >>> Fixes: signed integer overflow: 2314885530818453536 + 9151314442816847872 cannot be represented in type 'long'
> >>> Fixes: 68359/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-6571950311800832
> >>> 
> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>> libavformat/mov.c | 4 +++-
> >>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>> 
> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >>> index 9016cd5ad08..46cbce98040 100644
> >>> --- a/libavformat/mov.c
> >>> +++ b/libavformat/mov.c
> >>> @@ -8131,7 +8131,9 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >>>          }
> >>>          for (int j = 0; j < extent_count; j++) {
> >>>              if (rb_size(pb, &extent_offset, offset_size) < 0 ||
> >>> -                rb_size(pb, &extent_length, length_size) < 0)
> >>> +                rb_size(pb, &extent_length, length_size) < 0 ||
> >>> +                base_offset < 0 || extent_offset < 0 ||
> >>> +                base_offset + (uint64_t)extent_offset > INT64_MAX)
> >> 
> >> Can we please stop with the bespoke arithmetic overflow checks and add dedicated helpers instead, similar to what GCC and C23 have?
> >
> >You mean the __builtin_*_overflow() one?
> 
> I'd rather the ckd_*() stuff but the differences are mostly stylistic.

Whatever is used must be supported by all currently supported platforms
that especially also includes past releases we backport things to.

In practice that means continuing to use the classical way to check
as well as our av_sat_addXY() stuff.

We cannot backport things that depend on C23 as that was not a requirement
in the past. So I also cannot use this in bug fixes.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".