[FFmpeg-devel] [PATCH] fix vorbis decoder amplitude bits handling

[Yuriy Kaminskiy]

Alex Converse wrote:
> On Tue, Nov 16, 2010 at 2:42 AM, Yuriy Kaminskiy <yumkam at mail.ru> wrote:
>> Baptiste Coudurier wrote:
>>> Hi
>>>
>>> $subject. Please comment.
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> Index: libavcodec/vorbis_dec.c
>>> ===================================================================
>>> --- libavcodec/vorbis_dec.c   (revision 25756)
>>> +++ libavcodec/vorbis_dec.c   (working copy)
>>> @@ -563,6 +563,11 @@
>>>                       "Floor 0 amplitude bits is 0.\n");
>>>                return -1;
>>>              }
>>> +            if (floor_setup->data.t0.amplitude_bits > 32) {
>> Not enough, s/>/>=/, result of 1<<32 is undefined (and I'm sure code below
>> really broken on x86 when amplitude_bits == 32, resulting in divide-by-zero):
>> === cut ===
>>                    q = exp((((amplitude*vf->amplitude_offset) /
>>                              (((1 << vf->amplitude_bits) - 1) * sqrt(p + q)))
>>                             - vf->amplitude_offset) * .11512925f);
>> === cut ===
>>
> 
> Libvorbis does that. When in Rome...

Then libvorbis also have divide-by-zero error, someone should report them :-|
Besides, ampbits > 32 will result in reading outside of `mask' array inside
oggpack_read [in addition to producing totally bogus result].

Fortunately, floor0 in vorbis is very obsolete [it is only produced by libvorbis
before 1.0beta4 AFAIR] and rare; so I think it is totally safe to simply reject
files with ampbits >= 32.