[FFmpeg-devel] [PATCH 1/2] avcodec/bink: Fix integer overflow in unquantize_dct_coeffs()

Michael Niedermayer michael at niedermayer.cc
Sat Jun 22 09:46:20 EEST 2019


On Fri, Jun 21, 2019 at 09:12:36AM +0200, Reimar Döffinger wrote:
> 
> 
> On 18.06.2019, at 14:55, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > Fixes: signed integer overflow: -3447 * 2883584 cannot be represented in type 'int'
> > Fixes: 15265/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_BINK_fuzzer-5088311799971840
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> > libavcodec/bink.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/bink.c b/libavcodec/bink.c
> > index 8392bbeeb0..d18c0ceae4 100644
> > --- a/libavcodec/bink.c
> > +++ b/libavcodec/bink.c
> > @@ -702,15 +702,15 @@ static int read_dct_coeffs(BinkContext *c, GetBitContext *gb, int32_t block[64],
> >     return quant_idx;
> > }
> > 
> > -static void unquantize_dct_coeffs(int32_t block[64], const int32_t quant[64],
> > +static void unquantize_dct_coeffs(int32_t block[64], const uint32_t quant[64],
> >                                   int coef_count, int coef_idx[64],
> >                                   const uint8_t *scan)
> > {
> >     int i;
> > -    block[0] = (block[0] * quant[0]) >> 11;
> > +    block[0] = (int)(block[0] * quant[0]) >> 11;
> 
> Huh? How do you know the multiplication result will fit in an int?

its not known


> IIRC casting an out-of-range value to int is undefined behaviour, or does the tool fail to check that?
> I might miss something, but it looks to me like just replacing one undefined behaviour with another...

Its implementation defined and our codebase depends on the normal 
twos complement behavior of signed integers.

ISO/IEC 9899:2017   C17 ballot  N2176
6.3.1.3 Signed and unsigned integers
3 Otherwise, the new type is signed and the value cannot be represented in it; either the result is
implementation-defined or an implementation-defined signal is raised.


our developer.texi:
Implementation defined behavior for signed integers is assumed to match the
expected behavior for two's complement. Non representable values in integer
casts are binary truncated. Shift right of signed values uses sign extension.

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

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190622/56b6fe09/attachment.sig>


More information about the ffmpeg-devel mailing list