Possible overflow in Vorbis_cookbook.c

RESOLVED FIXED

Status

()

Core
Audio/Video
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: reed, Assigned: kinetik)

Tracking

({verified1.9.1})

unspecified
verified1.9.1
Points:
---
Bug Flags:
blocking1.9.2 +
wanted1.9.0.x -

Firefox Tracking Flags

(status1.9.2 beta1-fixed, blocking1.9.1 .4+, status1.9.1 .4-fixed)

Details

(Whiteboard: [sg:dupe 501279] already fixed upstream)

(Reporter)

Description

8 years ago
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).
(Reporter)

Updated

8 years ago
Whiteboard: [sg:critical?]
(Reporter)

Updated

8 years ago
Flags: blocking1.9.2?
Flags: blocking1.9.0.15?
(Reporter)

Updated

8 years ago
blocking1.9.1: --- → ?
Firefox 3.0 didn't have <video>, not needed for 1.9.0.x
status1.9.1: --- → wanted
Flags: blocking1.9.0.15? → wanted1.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.

Comment 3

8 years ago
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.
(Reporter)

Updated

8 years ago
Whiteboard: [sg:critical?] → [sg:critical] already fixed upstream
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)
blocking1.9.1: ? → .4+
Depends on: 501279
Whiteboard: [sg:critical] already fixed upstream → [sg:dupe 501279] already fixed upstream
(Reporter)

Updated

8 years ago
Assignee: nobody → kinetik
status1.9.1: wanted → .4-fixed
(Reporter)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
status1.9.2: --- → beta1-fixed
Flags: blocking1.9.2? → blocking1.9.2+
Is this marked fixed because the newer vorbis has been checked in?
Anyone want to answer my question?
(Assignee)

Comment 7

8 years ago
Yes.
Marking verified for 1.9.1 then.
Keywords: verified1.9.1

Comment 9

8 years ago
(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.
Group: core-security
You need to log in before you can comment on or make changes to this bug.