507030 - crash (segfault) @ vorbis_book_init_decode when playing corrupted ogg vorbis file

Status: RESOLVED WORKSFORME

(Core :: Audio/Video, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: test file that causes crash

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.12) Gecko/2009070811 Ubuntu/9.04 (jaunty) Firefox/3.0.12
Build Identifier: mozilla-central revision 7149d895e5b3

Segmentation fault when playing corrupted ogg vorbis file (attached).

Reproducible: Always

Steps to Reproduce:
1. Load attached file.
2. Possibly reload the page a couple of times.
Actual Results:  
firefox crashes

Expected Results:  
some sort of "this file is corrupted" message

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: stack trace of oggplayer

I wasn't able to get a stack trace from firefox for some reason, but here's the stack trace of oggplayer.

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler]]

In vorbis_sharedbook.c, we have the following:
(in vorbis_book_init_decode, starting at line 358)
ogg_uint32_t *codes=_make_words(s->lengthlist,s->entries,c->used_entries);
ogg_uint32_t **codep=alloca(sizeof(*codep)*n);
if(codes==NULL)goto err_out;

When running the given file, _make_words returns null, causing the program to jump to err_out and call vorbis_book_clear.  However, because n is very large, alloca has modified the stack pointer by more than 30 MB, which causes a segfault.
This is very probably exploitable (n is derived from the number of entries in the passed-in static_codebook, which presumably depends on the input file).
One really quick fix would be to move the null check to above the call to alloca, but this doesn't get rid on the underlying problem, which is the unsafe use of alloca.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Bug poke: is this being worked on?
I could write the patch for the quick fix I proposed above, but I'm not sure what to do about the alloca calls.  Should they just be changed to malloc? (with appropriate calls to free, of course)

Comment 4

[cajbir (:cajbir)]

xiphmont, the author of the code, is cc'd on the bug. I'm interested in what his thoughts on a fix are,

Comment 5

[cajbir (:cajbir)]

This does not crash on trunk for me.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

WFM [Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091020 Minefield/3.7a1pre].

File loads, but won't play. Would be a good testcase/crashtest.