#421 malformed .flac file causes crash / segfault

[Hanno Böck]

Attached file causes a segfault in flac (flac -df crash.flac).

Here's a backtrace from gdb:
Program received signal SIGSEGV, Segmentation fault.
0x000000000043d0df in read_metadata_ (decoder=decoder@entry=0x962350) at stream_decoder.c:1536
1536 if(0 != block.data.vorbis_comment.comments[i].entry)
(gdb) bt

0 0x000000000043d0df in read_metadata_ (decoder=decoder@entry=0x962350) at stream_decoder.c:1536

1 0x000000000043ef38 in FLAC__stream_decoder_process_until_end_of_metadata (decoder=0x962350)

at stream_decoder.c:1065

2 0x000000000040b291 in DecoderSession_process (d=<optimized out="">) at decode.c:339</optimized>

3 flac__decode_file (infilename=0x9620f0 "bar.flac", outfilename=0x7fffffffd4a0 "", analysis_mode=0,

aopts=..., options=...) at decode.c:180

4 0x0000000000402a9c in decode_file (infilename=<optimized out="">) at main.c:2156</optimized>

5 0x0000000000405823 in do_it () at main.c:499

6 main (argc=<optimized out="">, argv=<optimized out="">) at main.c:333</optimized></optimized>

This issue was found with the help of american fuzzy lop.

[Hanno Böck]

This was my fuzzing input

[Erik]

Ok, problem with this file is that it says it has 1701209458 vorbis comments in it and address-sanitizer segfaults when an attempt is made to allocate memory for that number of comments.

The solution seems to be to set an upper bound on the number of allowed comments.

[Hanno Böck]

An upper bound for the comments doesn't seem like a clean solution

There's something odd here. Looking at the code and different debugging output this shouldn't happen.
The gdb backtrace indicates already an access to the non-allocated memory (stream_decoder.c, line 1732). But it shouldn't reach that code if the memory allocation error is properly checked.

Appart from that I think to avoid the too large allocation from happening at all one should do some sanity checks: If the number of comments * minimal comment size is larger than the file an error should be thrown.

[Erik]

If the file is being streamed, the stream decoder can't know the length of the file at the time it reads the number of vorbis comments.

[Erik]

In stream_decoder.c, line 1732, with a obj->num_comments value of 1701209458, the function safe_malloc_mul_2op_p returns NULL, which gets returned to function read_metadata_ which assumes that the memory has been allocated.

My current solution to this problem is to set obj->num_comments to zero if safe_malloc_mul_2op_p returns NULL.

[Erik]

@Hanno it would also be useful if you could tell me everything about how you set up and ran afl. I still can't reproduce your results.

[Hanno Böck]

Probably something like this:
a) export AFL_HARDEN=1; export AFL_USE_ASAN=1
b) compiled flac with ./configure --disable-shared CC=afl-gcc CXX=afl-g++; make
c) copied supplied mini flac input to directory ./in/
d) afl-fuzz -m -1 -i in -o out [path_to]/flac -df @@

To explain: AFL_HARDEN enables some compile flags to expose more bugs, AFL_USE_ASAN enables address sanitizer. -m -1 disables the memory limit. flac -d (decoder) and -f (overwrite output files, because we'll run flac thousands of times), @@ is replaced with the input filename. --disable-shared makes sure that we don't use the system-wide libflac.

I can reliably reproduce this. System is Gentoo, afl is 0.74b, gcc is 4.9.2.

[Erik]

Awesome thanks! I can now crash flac. Looks like I'll be busy this weekend.

[Erik]

Fixed in this commit:

Author: Erik de Castro Lopo <erikd@mega-nerd.com>
Date:   Fri Nov 28 23:39:25 2014 +1100

src/libFLAC/stream_decoder.c : Fix another input validation bug.

If a file says it contains a stupidly large number of vorbis comments,
the stream decoder would try to allocate enough memory which would fail
returning NULL and then write to that pointer anyway. The solution is
to set a hard limit of 10000 vorbis comments and force num_comments to
zero if the number is too large.

Problem found using the afl (american fuzzy lop) fuzzer.

Closes: https://sourceforge.net/p/flac/bugs/421/
Reported-by : Hanno Böck <hanno@hboeck.de>