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

[Tomas Härdin]

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
for av_calloc()? Or do it manually less uglily like so:

  index > INT64_MAX / s->edit_unit_byte_count

> +                    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


/Tomas