[FFmpeg-devel] [PATCH 2/2] avformat/matroskadec: avoid integer overflows in SAR computation

[Michael Niedermayer]

On Tue, Apr 05, 2022 at 11:59:44AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2022-04-01 12:46:08)
> > This ignores >64bit
> > Alternatively we could support that if it occurs in reality
> > 
> > Fixes: negation of -9223372036854775808
> > Fixes: integer overflows
> > Fixes: 46072/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-5029840966778880
> > 
> > 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/matroskadec.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index d97fc33d44..069fba6cf7 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -2886,11 +2886,14 @@ static int matroska_parse_tracks(AVFormatContext *s)
> >                  mkv_stereo_mode_display_mul(track->video.stereo_mode, &display_width_mul, &display_height_mul);
> >  
> >              if (track->video.display_unit < MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) {
> > -                av_reduce(&st->sample_aspect_ratio.num,
> > -                          &st->sample_aspect_ratio.den,
> > -                          st->codecpar->height * track->video.display_width  * display_width_mul,
> > -                          st->codecpar->width  * track->video.display_height * display_height_mul,
> > -                          INT_MAX);
> > +                if (track->video.display_width && track->video.display_height &&
> > +                    st->codecpar->height * (int64_t)display_width_mul  < INT64_MAX / track->video.display_width &&
> > +                    st->codecpar->width  * (int64_t)display_height_mul < INT64_MAX / track->video.display_height)
> 
> Why not move display_{width,height}_mul to the other side of the
> comparison and avoid wordsize assumptions? This is header parsing, so
> division performance impact should be negligible.

will do

thx

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

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.
-------------- 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/20220405/d2c82a82/attachment.sig>