opus_tags_parse_impl: Fix free of uninitialised data
As part of our fuzzing efforts at Google, we have identified an issue in git HEAD of opusfile. To reproduce requires compiling the project with the LLVM compiler, taking advantage of the sanitizers that it offers (this issue was discovered using AddressSanitizer.
To reproduce you will need to build your project using that sanitizer, and execute the attached stub code on the reproducer input that we have also provided. This stub code could also serve as a useful template for fuzzing in your project with llibFuzzer and/or AFL, which may help you uncover additional issues. Some documentation on how to get started with libFuzzer is here:
The following options / environment variables may be necessary for accurate reproduction of the issue as well:
ASAN_OPTIONS="exitcode=1,handle_segv=1,detect_leaks=1,leak_check_at_exit=1,a llocator_may_return_null=1,detect_odr_violation=0"
MSAN_OPTIONS=...
The sanitizer error that we encountered is here:
==834381==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000448ea4 bp 0x7fff554ee620 sp 0x7fff554ee5f0 T0)
==834381==The signal is caused by a READ memory access.
==834381==Hint: address points to the zero page.
Other relevant info/repro instructions:
The crash occurs during parsing of invalid user_comments tags.
I configured with:
CC=clang-3.6 CFLAGS="-g -fsanitize=address -fno-omit-frame-pointer" ./configure --enable-static --disable-shared
And built with:
clang-3.6 -fno-omit-frame-pointer -g -fsanitize=address -Iinclude -I /usr/include/opus/ -L .libs -o poc poc.c -lopusfile -logg -lopus
I'm not familiar with opusfile, but I think the chain of events is:
1. opus_tags_parse() is called, calls opus_tags_parse_impl()
2. opus_tags_parse_impl() starts parsing, gets an okay preamble and comment count of 1
3. opus_tags_parse_impl() calls op_tags_ensure_capacity() to allocate _tags->user_comments
4. opus_tags_parse_impl() user_comments parsing loop encounters a comment with a nonsensical length, returns with error *without* initialising the comment.
5. opus_tags_parse() calls opus_tags_clear() which has this test:
if(_tags->user_comments!=NULL)ncomments++;
for(ci=ncomments;ci-->0;)_ogg_free(_tags->user_comments[ci]);
It assumes that if _tags->user_comments was allocated, then its contents are valid but this isn't the case because of #4.
Always initialising _tags->user_comments[ci] in opus_tags_parse_impl() should be sufficient fix this.
Suggested fix in libopusfile/src/info.c:
if(_tags!=NULL) _tags->user_comments[ci]=NULL;
We will gladly work with you so you can successfully confirm and reproduce this issue. Do let us know if you have any feedback surrounding the documentation.
Once you have reproduced the issue, we’d appreciate to learn your expected timeline for an update to be released. With any fix, please attribute the report to “Google Autofuzz project”.
We are also pleased to inform you that your project is also eligible for inclusion to the OSS-Fuzz project, which can provide additional continuous fuzzing, and encourage you to investigate integration options.
Don’t hesitate to let us know if you have any questions!
Google AutoFuzz Team