[FFmpeg-devel] stsz overflow

[Baptiste Coudurier]

On 8/25/2009 9:16 AM, Reimar D?ffinger wrote:
> 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.

Huh, IMHO changing one if is not additional code and not complex.
Besides both approaches were ok'd, Alex chose this one.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org