[FFmpeg-devel] [PATCH] H.264 timestamps in h264_parser - complete set

[Ivan Schreter]

Michael Niedermayer wrote:
> On Wed, Feb 18, 2009 at 12:33:57PM +0100, Ivan Schreter wrote:
>   
> [...]
>   
>>> [...]
>>>
>>>   
>>>       
>>>> @@ -6859,6 +6860,37 @@
>>>>      return 0;
>>>>  }
>>>>
>>>> +static int decode_buffering_period(H264Context *h){
>>>> +    MpegEncContext * const s = &h->s;
>>>> +    int sps_id;
>>>> +    int sched_sel_idx;
>>>> +    SPS *sps;
>>>> +
>>>> +    sps_id = get_ue_golomb_31(&s->gb);
>>>>     
>>>>         
>>> this is missing a validity check (<32 i suspect but didnt check)
>>> also as this would have been possibly exploitable, please be carefull not 
>>> to
>>> miss such checks
>>>   
>>>       
>> According to docs of get_ue_golomb_31(), it can only return value in range 
>> 0..31. SPS ID can be in range 0..31 as well, so no check required. However, 
>> looking at get_ue_golomb_31() code, the lookup table contains also return 
>> value of 32! So either doc is wrong or the lookup table is wrong. I've 
>> added the check to be on the safe side.
>>     
>
> fixed doc, and you need to make the check unsigned
>   
Why? The function returns int in range 0..32. It takes 9 bits from 
current word and uses this as an index into lookup array containing 
values 0..32. So it cannot possibly return negative value.

> [...]
> no initial_cpb_removal_delay_offset is still there
>
>   
Sorry, forgot to replace the last hunk. That's the problem with having 
too many active changes :-(

Attached fixed #6.

[...]

Regards,

Ivan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: h264_timing_6_buffering_period.patch
Type: text/x-patch
Size: 3489 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090218/cf346864/attachment.bin>