[FFmpeg-devel] [PATCH 3/3] avformat/mxfdec: Check first case of offset_temp computation for overflow

[Michael Niedermayer]

On Sat, Mar 30, 2024 at 10:01:32AM +0100, Tomas Härdin wrote:
> fre 2024-03-29 klockan 20:32 +0100 skrev Michael Niedermayer:
> > This is kind of ugly
> > Fixes: signed integer overflow: 255 * 1157565362826411919 cannot be
> > represented in type 'long'
> > Fixes: 67313/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> > 6250434245230592
> > 
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavformat/mxfdec.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index c9af4628555..fe86f516630 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -1891,9 +1891,14 @@ static int
> > mxf_edit_unit_absolute_offset(MXFContext *mxf, MXFIndexTable *index_t
> >          if (edit_unit < s->index_start_position + s->index_duration)
> > {
> >              int64_t index = edit_unit - s->index_start_position;
> >  
> > -            if (s->edit_unit_byte_count)
> > +            if (s->edit_unit_byte_count) {
> > +                if (s->edit_unit_byte_count * (uint64_t)index / s-
> > >edit_unit_byte_count != index ||
> 
> Don't we already have a function for testing these kinds of overflows

We have av_size_mult() thats for size_t


> for av_calloc()? Or do it manually less uglily like so:
> 
>   index > INT64_MAX / s->edit_unit_byte_count

ok

> 
> > +                    s->edit_unit_byte_count * index > INT64_MAX -
> > offset_temp
> > +                )
> 
> Nit: looks a bit weird to have the ) there rather than at the end of
> the previous line

will push with this changed

thx

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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
than the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240401/5028e225/attachment.sig>