| Message ID | 20180119224808.5055-1-nbowe@google.com |
|---|---|
| State | New |
| Headers | show |
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 [...]
On Fri, Jan 19, 2018 at 3:00 PM, Michael Niedermayer <michael@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
On Fri, Jan 19, 2018 at 03:56:19PM -0800, Niki Bowe wrote: > On Fri, Jan 19, 2018 at 3:00 PM, Michael Niedermayer <michael@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? correct range checks would be ideal. if that is not possible then at least bit_rate should not be set "randomly" in case of an overflow thx [...]
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;