[FFmpeg-cvslog] r18088 - trunk/libavformat/mxfdec.c

[Baptiste Coudurier]

On 3/22/2009 2:31 AM, Reimar D?ffinger wrote:
> On Sat, Mar 21, 2009 at 08:20:43PM -0700, Baptiste Coudurier wrote:
>> bcoudurier wrote:
>>> Author: bcoudurier
>>> Date: Sat Mar 21 01:50:51 2009
>>> New Revision: 18088
>>>
>>> Log:
>>> protect realloc overflow
>>>
>>> Modified:
>>>    trunk/libavformat/mxfdec.c
>>>
>>> Modified: trunk/libavformat/mxfdec.c
>>> ==============================================================================
>>> --- trunk/libavformat/mxfdec.c	Sat Mar 21 01:50:19 2009	(r18087)
>>> +++ trunk/libavformat/mxfdec.c	Sat Mar 21 01:50:51 2009	(r18088)
>>> @@ -364,6 +364,8 @@ static int mxf_read_primer_pack(MXFConte
>>>  
>>>  static int mxf_add_metadata_set(MXFContext *mxf, void *metadata_set)
>>>  {
>>> +    if (mxf->metadata_sets_count+1 >= UINT_MAX / sizeof(*mxf->metadata_sets))
>>> +        return AVERROR(ENOMEM);
>>>      mxf->metadata_sets = av_realloc(mxf->metadata_sets, (mxf->metadata_sets_count + 1) * sizeof(*mxf->metadata_sets));
>>>      if (!mxf->metadata_sets)
>>>          return -1;
>> Just to be sure, is the test sufficient and the best ?
> 
> Depends is metadata_sets_count int or unsigned?

int

> The +1 can most likely overflow, simple rule: a working overflow check
> will almost always have the check variable standing alone on one side.
> Writing the equation the naive way and moving everything else to the other
> side often works, you just have to make sure rounding happens the right
> way around.

the +1 yes, however the check is >=, so it should be safe no ? It would
just fail one before the max, if I'm not mistaken.
What would be the best situation ? unsigned and > ? (unsigned) cast + 1
and >= ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org