| Message ID | 20220401104608.10193-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/wmalosslessdec: Check channels after setting them | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
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@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.
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@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 [...]
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) + 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 (st->codecpar->codec_id != AV_CODEC_ID_HEVC) sti->need_parsing = AVSTREAM_PARSE_HEADERS;
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@niedermayer.cc> --- libavformat/matroskadec.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)