[FFmpeg-devel] stsz overflow

[Reimar Döffinger]

On Tue, Aug 25, 2009 at 09:11:21AM -0700, Baptiste Coudurier wrote:
> On 8/25/2009 12:26 AM, Reimar D?ffinger wrote:
> > On Mon, Aug 24, 2009 at 07:05:53PM -0700, Frank Barchard wrote:
> >> On Mon, Aug 24, 2009 at 4:08 PM, Alex Converse<alex.converse at gmail.com>wrote:
> >>> The intermediate product here is the part that overflows. A final
> >>> num_bytes calculated with appropriate intermediate precision should
> >>> fit in in an unsigned 32-bit integer. Why not just fix that rather
> >>> than reduce the number of entries supported?
> >>
> >> Alex,
> >> Sorry, thats not going true overflows, where the final num_bytes is>
> >> MAX_INT
> >> Also this expression will overflow.
> >> init_get_bits(&gb, buf, 8*num_bytes);
> >>
> >> This patch uses uint64_t to avoid math overflow, but checks the size before
> >> attempting the av_malloc()
> >
> > Wow, what a mess (IMO). I think we are already at the point where it
> > would be simpler to just get rid of that buffer and directly read the
> > values "one by one" from the file.
> 
> No, it was decided to be done that way when the patch was submitted.

Obviously the review at that time did not take into account the
additional code and complexity of avoiding buffer overflows, which
unless someone comes up with a cleaner check is a really good reason
to reconsider that decision.