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

[Michael Niedermayer]

On Wed, Feb 18, 2009 at 01:41:27PM +0100, Ivan Schreter wrote:
> 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.

the function returns an undefined value if the bitstream is not storing 0..31

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090218/5627a6b0/attachment.pgp>