Last Comment Bug 515889 - Possible overflow in Vorbis_cookbook.c
: Possible overflow in Vorbis_cookbook.c
[sg:dupe 501279] already fixed upstream
: verified1.9.1
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: Matthew Gregan [:kinetik]
: Maire Reavy [:mreavy] Please needinfo me
Depends on: CVE-2009-3379
  Show dependency treegraph
Reported: 2009-09-10 23:12 PDT by Reed Loden [:reed] (use needinfo?)
Modified: 2009-11-09 18:35 PST (History)
12 users (show)
roc: blocking1.9.2+
dveditz: wanted1.9.0.x-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description User image Reed Loden [:reed] (use needinfo?) 2009-09-10 23:12:45 PDT
Dan Kaminsky reported an issue with Vorbis to Monty of that could possibly affect Firefox.


    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;
   case 0:
   case 1:
   /* Do we have a mapping to unpack? */
   case 0:
   case 1: case 2:
       case 1:
       case 2:
     quantvals=s->entries*s->dim; // 2**16-1 * 2**24-1 overflows, and probably affects things elsewhere in code, but...

       /* quantized values */
       s->quantlist=_ogg_malloc(sizeof(*s->quantlist)*quantvals); // this is the useful overflow
     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).
Comment 1 User image Daniel Veditz [:dveditz] 2009-09-11 10:30:16 PDT
Firefox 3.0 didn't have <video>, not needed for 1.9.0.x
Comment 2 User image Samuel Sidler (old account; do not CC) 2009-09-11 10:32:40 PDT
Need comments here on if this is actually exploitable before we know whether we need to block on it for 1.9.1.
Comment 3 User image Dan Kaminsky 2009-09-11 11:35:37 PDT
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 .

Should definitely block on this one.  Adding Tim for his comments.
Comment 4 User image Daniel Veditz [:dveditz] 2009-09-14 17:57:23 PDT
Bug 501279 incorporated the newer vorbis into 1.9.2

We're working on getting that into 1.9.1.x (Firefox 3.5.x)
Comment 5 User image Al Billings [:abillings] 2009-09-29 10:59:05 PDT
Is this marked fixed because the newer vorbis has been checked in?
Comment 6 User image Al Billings [:abillings] 2009-10-16 16:13:06 PDT
Anyone want to answer my question?
Comment 7 User image Matthew Gregan [:kinetik] 2009-10-16 17:14:22 PDT
Comment 8 User image Al Billings [:abillings] 2009-10-16 17:18:49 PDT
Marking verified for 1.9.1 then.
Comment 9 User image Tomas Hoger 2009-10-29 06:54:54 PDT
(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 .

... which was added in libvorbis SVN r14604 and is also known as CVE-2008-1423 (previously reported by Will Drewry), unless I'm mistaken.

Note You need to log in before you can comment on or make changes to this bug.