We've been working on a draft spec for encapsulation of FLAC
in the ISO Base Media File Format (mp4). This is the initial
draft created by Monty Montgomery based on Yusuke Nakamura's
Opus-in-mp4 draft.
More details at https://bugzilla.mozilla.org/show_bug.cgi?id=1286097
Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
If a seek table has already been read successfully, then the
has_seek_table flag is true. Now imagine the file comes with another
seek table, which doesn't make sense, but libFLAC accepts it happily.
If reading this second seek table fails (for example allocation
failure), read_metadata_seektable_() returns false, but the
has_seek_table flag is still true. If the calling application happens
to ignore this failure, and at some point tries to seek, the process
will crash due to NULL pointer dereference. This would sure be an
application bug that needs to be fixed, but libFLAC's internal state
is inconsistent, so let's fix this up.
Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
When read_metadata_seektable_() fails, the has_seek_table flag is
never set to true, and thus free() is never called.
Example valgrind output:
11,185,464 bytes in 1 blocks are definitely lost in loss record 62 of 62
at 0x4C2BC0F: malloc (vg_replace_malloc.c:299)
by 0x4C2DE6F: realloc (vg_replace_malloc.c:785)
by 0x40A7880: safe_realloc_ (alloc.h:159)
by 0x40A7911: safe_realloc_mul_2op_ (alloc.h:205)
by 0x40AB6B5: read_metadata_seektable_ (stream_decoder.c:1654)
by 0x40AAB2D: read_metadata_ (stream_decoder.c:1422)
by 0x40A9C79: FLAC__stream_decoder_process_until_end_of_metadata (stream_decoder.c:1055)
It is easy to craft a FLAC file which leaks megabytes of memory on
every attempt to open the file.
This patch fixes the problem by removing checks which are unnecessary
(and harmful). Checking the has_seek_table flag is not enough, as
described above. The NULL check is not harmful, but is not helpful
either, because free(NULL) is documented to be legal.
After running this code block, we're in a well-known safe state, no
matter how inconsistent pointer and flag may have been before, for
whatever reasons.
Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
Some old CPUs (eg Cyrix) set EDX but not ECX when executing CPUID.
One of the solutions is to clear ECX before calling cpuid. From
https://bugzilla.mozilla.org/show_bug.cgi?id=1096651#c9 bug
Patch-from: lvqcl <lvqcl.mail@gmail.com>
Previously, it the write callback failed the error status
would be set to `FLAC__STREAM_DECODER_READ_FRAME`. Now it
gets set to `FLAC__STREAM_DECODER_WRITE_STATUS_ABORT`.
Patch-from: lvqcl <lvqcl.mail@gmail.com>
These asserts were being triggered by AFL (American Fuzzy Lop) and
serve seemingly no useful purpose. The are only enabled in debug builds
where they abort the program which is otherwise in a safe state.
Removing these asserts will potentially allow AFL to turn up other
problems elsewhere.
It is pretty easy for a malformed FLAC file to underflow the "bps"
variable. In the debug build, this results in an assertion failure in
FLAC__bitreader_read_raw_uint32():
FLAC__ASSERT(bits <= 32);
In non-debug builds, this simply makes
FLAC__bitreader_read_raw_uint32() fail because
bitreader_read_from_client_() doesn't find enough buffer space for
2**32-1 bits. But since the failing FLAC_ASSERT() is reasonable, this
should be caught in the FLAC__bitreader_read_raw_uint32() caller.
Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
Closes: https://github.com/xiph/flac/pull/13
FLAC__stream_decoder_process_single() ignores frame_sync_() errors,
which means the caller cannot rely solely on the boolean return value,
it is also required to check the new "state".
After FLAC__stream_decoder_process_until_end_of_metadata(),
state==SEARCH_FOR_FRAME_SYNC and
last_frame.header.number_type==FRAME_NUMBER. When an application
seeks at this time, but an I/O error occurs, then
FLAC__stream_decoder_process_single() returns true, but no frame has
been read yet, i.e. last_frame.header.number_type is still
FRAME_NUMBER. This triggers the assertion in
seek_to_absolute_sample_():
FLAC__ASSERT(decoder->private_->last_frame.header.number_type == FLAC__FRAME_NUMBER_TYPE_SAMPLE_NUMBER);
So what needs to be done is check for state==ABORTED after the
FLAC__stream_decoder_process_single() call.
This bug can be triggered remotely with the Music Player Daemon
(https://www.musicpd.org/), and crashes the process.
Signed-off-by: Erik de Castro Lopo <erikd@mega-nerd.com>
Closes: https://github.com/xiph/flac/pull/12
Patch pulled from Debian package.
Description:
Chain::Status::as_cstring uses FLAC__Metadata_ChainStatusString which
is in libFLAC. Since the function is inline, every program calling
this function must also link with -lflac, but this is missing in
flac++.pc.
Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=713645
Erik de Castro Lopo