[FFmpeg-devel] Neither vorbis_parse_setup_hdr_codebooks nor ff_vorbis_len2vlc verify data

Reimar Döffinger Reimar.Doeffinger
Wed Jul 8 10:52:42 CEST 2009


On Wed, Jul 08, 2009 at 02:26:50AM +0200, Michael Niedermayer wrote:
> On Sun, Jul 05, 2009 at 10:21:01AM +0200, Reimar D?ffinger wrote:
> > On Sun, Jul 05, 2009 at 10:18:02AM +0200, Reimar D?ffinger wrote:
> > > sample is ogv/smclock.ogv.2.164.ogv from issue 1240.
> > > vorbis_parse_setup_hdr_codebooks can at least create values up to 33
> > > (get_bits(gb, 5)+1)
> 
> am i missing something? get_bits(gb, 5)+1 should be 32 max not 33

No, I am silly.

> > > ff_vorbis_len2vlc which these values are then passed to on the other
> > > hand just assumes that the bits values are at most 32, otherwise it just
> > > writes beyond the exit_at_level and onto the stack, overwriting the
> > > return address.
> > > Since there is no documentation for the function the question is which
> > > is wrong? Should ff_vorbis_len2vlc have a check or should
> > > vorbis_parse_setup_hdr_codebooks?
> 
> i would guess that vorbis_parse_setup_hdr_codebooks() is the one that needs
> a check as one side of the ordered branch can reach higher values, this
> dosent look too right but its just my gut feeling.

Well, this one seems to work for the sample at least:

Index: vorbis_dec.c
===================================================================
--- vorbis_dec.c        (revision 19365)
+++ vorbis_dec.c        (working copy)
@@ -292,7 +292,7 @@
             AV_DEBUG(" ordered, current length: %d \n", current_length);  //FIXME
 
             used_entries=entries;
-            for(;current_entry<used_entries;++current_length) {
+            for(;current_entry<used_entries && current_length <= 32;++current_length) {
                 uint_fast16_t i, number;
 
                 AV_DEBUG(" number bits: %d ", ilog(entries - current_entry));

Though I am not 100% confident this is all that is needed, so I'd prefer
to apply the other one at least in addition.



More information about the ffmpeg-devel mailing list