| Message ID | ad3fbd69-c415-edc3-569d-76f8d5017f4c@googlemail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 14, 2016 at 12:57:12AM +0100, Andreas Cadhalpun wrote: > On 13.12.2016 01:32, Michael Niedermayer wrote: > > On Tue, Dec 13, 2016 at 12:50:19AM +0100, Andreas Cadhalpun wrote: > >> The arguments of av_reduce are signed, so the cast to uint64_t is misleading. > >> > >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > >> --- > >> libavcodec/tiff.c | 11 +++++++++-- > >> 1 file changed, 9 insertions(+), 2 deletions(-) > >> > >> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c > >> index 4721e94..12ef419 100644 > >> --- a/libavcodec/tiff.c > >> +++ b/libavcodec/tiff.c > >> @@ -772,9 +772,16 @@ static void set_sar(TiffContext *s, unsigned tag, unsigned num, unsigned den) > >> int offset = tag == TIFF_YRES ? 2 : 0; > >> s->res[offset++] = num; > >> s->res[offset] = den; > >> - if (s->res[0] && s->res[1] && s->res[2] && s->res[3]) > >> + if (s->res[0] && s->res[1] && s->res[2] && s->res[3]) { > >> + uint64_t num = s->res[2] * (uint64_t)s->res[1]; > >> + uint64_t den = s->res[0] * (uint64_t)s->res[3]; > >> + if (num > INT64_MAX || den > INT64_MAX) { > >> + num = num >> 1; > >> + den = den >> 1; > >> + } > > > > this can make one of them 0, in fact i think even if they arent 0 > > the sample_aspect_ratio can be after reduce > > should they be checked after all that instead of before ? > > I've added a check for !s->avctx->sample_aspect_ratio.den after av_reduce. > The check before is still necessary to prevent sample_aspect_ratio from > becoming negative. > > Best regards, > Andreas > > tiff.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > 96297593c98fcbce7395cb13627fae080f1e2cbc 0001-tiff-fix-overflows-when-calling-av_reduce.patch > From 3cd8cb663d762bc15694e285ea48cdb8e9abfd4b Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > Date: Tue, 13 Dec 2016 00:43:21 +0100 > Subject: [PATCH] tiff: fix overflows when calling av_reduce > > The arguments of av_reduce are signed, so the cast to uint64_t is misleading. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavcodec/tiff.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) LGTM thx [...]
On 14.12.2016 13:34, Michael Niedermayer wrote: > On Wed, Dec 14, 2016 at 12:57:12AM +0100, Andreas Cadhalpun wrote: >> tiff.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> 96297593c98fcbce7395cb13627fae080f1e2cbc 0001-tiff-fix-overflows-when-calling-av_reduce.patch >> From 3cd8cb663d762bc15694e285ea48cdb8e9abfd4b Mon Sep 17 00:00:00 2001 >> From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> Date: Tue, 13 Dec 2016 00:43:21 +0100 >> Subject: [PATCH] tiff: fix overflows when calling av_reduce >> >> The arguments of av_reduce are signed, so the cast to uint64_t is misleading. >> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> --- >> libavcodec/tiff.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) > > LGTM Pushed. Best regards, Andreas
From 3cd8cb663d762bc15694e285ea48cdb8e9abfd4b Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> Date: Tue, 13 Dec 2016 00:43:21 +0100 Subject: [PATCH] tiff: fix overflows when calling av_reduce The arguments of av_reduce are signed, so the cast to uint64_t is misleading. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/tiff.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c index 4721e94..7ccda51 100644 --- a/libavcodec/tiff.c +++ b/libavcodec/tiff.c @@ -772,9 +772,18 @@ static void set_sar(TiffContext *s, unsigned tag, unsigned num, unsigned den) int offset = tag == TIFF_YRES ? 2 : 0; s->res[offset++] = num; s->res[offset] = den; - if (s->res[0] && s->res[1] && s->res[2] && s->res[3]) + if (s->res[0] && s->res[1] && s->res[2] && s->res[3]) { + uint64_t num = s->res[2] * (uint64_t)s->res[1]; + uint64_t den = s->res[0] * (uint64_t)s->res[3]; + if (num > INT64_MAX || den > INT64_MAX) { + num = num >> 1; + den = den >> 1; + } av_reduce(&s->avctx->sample_aspect_ratio.num, &s->avctx->sample_aspect_ratio.den, - s->res[2] * (uint64_t)s->res[1], s->res[0] * (uint64_t)s->res[3], INT32_MAX); + num, den, INT32_MAX); + if (!s->avctx->sample_aspect_ratio.den) + s->avctx->sample_aspect_ratio = (AVRational) {0, 1}; + } } static int tiff_decode_tag(TiffContext *s, AVFrame *frame) -- 2.10.2