Bugzilla@Mozilla – Bug 515889
Possible overflow in Vorbis_cookbook.c
Last modified: 2009-11-09 18:35:32 PST
Summon comment box
Dan Kaminsky reported an issue with Vorbis to Monty of xiph.org that could possibly affect Firefox. -------------------- Monty, Looking through your code as embedded in FF3.5, I see in Vorbis_cookbook.c something that might be a problem: /* unpacks a codebook from the packet buffer into the codebook struct, readies the codebook auxiliary structures for decode *************/ int vorbis_staticbook_unpack(oggpack_buffer *opb,static_codebook *s){ ... /* first the basic parameters */ s->dim=oggpack_read(opb,16); // unbound, 0 to 2**16-1 s->entries=oggpack_read(opb,24); // unbound, 0 to 2**24-1 (must not be 1) if(s->entries==-1)goto _eofout; ... switch((int)oggpack_read(opb,1)){ case 0: ... case 1: ... /* Do we have a mapping to unpack? */ switch((s->maptype=oggpack_read(opb,4))){ case 0: ... case 1: case 2: switch(s->maptype){ case 1: ... case 2: quantvals=s->entries*s->dim; // 2**16-1 * 2**24-1 overflows, and probably affects things elsewhere in code, but... break; } /* quantized values */ s->quantlist=_ogg_malloc(sizeof(*s->quantlist)*quantvals); // this is the useful overflow for(i=0;i<quantvals;i++) s->quantlist[i]=oggpack_read(opb,s->q_quant); //will read arbitrary values into RAM, 41 41 00 00 style. Mind taking a look? Attacker seems to have two options; first, write lots of memory, second, make quantvals negative thus leaving the quantization tables unallocated (and skipping any likely AV's).
Firefox 3.0 didn't have <video>, not needed for 1.9.0.x
Need comments here on if this is actually exploitable before we know whether we need to block on it for 1.9.1.
No, this one's pretty clean, the overflow, alloc, and copy from attacker controlled content all happen in one function with no other checks that could possibly interfere. This is *also* fixed upstream in Xiph code, with: if(_ilog(s->dim)+_ilog(s->entries)>24)goto _eofout; ...inside of http://svn.xiph.org/trunk/vorbis/lib/codebook.c . Should definitely block on this one. Adding Tim for his comments.
Bug 501279 incorporated the newer vorbis into 1.9.2 http://mxr.mozilla.org/mozilla1.9.2/source/media/libvorbis/lib/vorbis_codebook.c#149 We're working on getting that into 1.9.1.x (Firefox 3.5.x)
Is this marked fixed because the newer vorbis has been checked in?
Anyone want to answer my question?
Yes.
Marking verified for 1.9.1 then.
(In reply to comment #3) > This is *also* fixed upstream in Xiph code, with: > > if(_ilog(s->dim)+_ilog(s->entries)>24)goto _eofout; > > ...inside of http://svn.xiph.org/trunk/vorbis/lib/codebook.c . ... which was added in libvorbis SVN r14604 and is also known as CVE-2008-1423 (previously reported by Will Drewry), unless I'm mistaken.