[FFmpeg-devel] [PATCH] Fix signed integer overflow undefined behavior

Niki Bowe nbowe at google.com
Sat Jan 20 01:56:19 EET 2018


On Fri, Jan 19, 2018 at 3:00 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Fri, Jan 19, 2018 at 02:48:08PM -0800, Nikolas Bowe wrote:
> > Found via fuzzing
> > ---
> >  libavformat/rpl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/rpl.c b/libavformat/rpl.c
> > index d373600478..df449bfc29 100644
> > --- a/libavformat/rpl.c
> > +++ b/libavformat/rpl.c
> > @@ -194,7 +194,7 @@ static int rpl_read_header(AVFormatContext *s)
> >          if (ast->codecpar->bits_per_coded_sample == 0)
> >              ast->codecpar->bits_per_coded_sample = 4;
> >
> > -        ast->codecpar->bit_rate = ast->codecpar->sample_rate *
> > +        ast->codecpar->bit_rate = (uint64_t)ast->codecpar->sample_rate
> *
> >                                    ast->codecpar->bits_per_coded_sample
> *
> >                                    ast->codecpar->channels;
>
> uint64_t is the wrong type, bit_rate is int64_t
>
>
That is true, but wouldn't fix the overflow
signed int64_t can still overflow eg if sample_rate, bits_per_coded_sample,
and channels are all INT_MAX.
By using uint64_t, the overflow is at least defined behavior.

It might be better to range check these values, but I could not find a
comprehensive spec.
I could use reasonably lenient values of say 384 khz and 32 bits per coded
sample.
Would you prefer that?



> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


-- 

Nikolas Bowe |  SWE |  nbowe at google.com |  408-565-5137


More information about the ffmpeg-devel mailing list