[flac-dev] Two new CVEs against FLAC

[Miroslav Lichvar]

On Wed, Dec 10, 2014 at 10:54:15PM -0800, Erik de Castro Lopo wrote:
> Erik de Castro Lopo wrote:
> 
> > I think I have an alternative fix for the CVE which should not break
> > seeking. I'm working on getting an copy of the file with which to test.
> 
> Patch applied and pushed.

I think this revives the CVE, at least in some configurations. The
patch seems to cover only the problem with memcpy writing past the
buffer when blocksize is smaller than order, but not the unbound LPC
decoding with functions that don't use data_len as a signed value,
e.g. FLAC__lpc_restore_signal_16_intrin_sse2 for orders between 8 and
12, or the non-unrolled version of FLAC__lpc_restore_signal.

The code below should catch that, but I'd rather see the real seeking
bug fixed instead and not hide it like this. Returning success with
invalid/uninitialized data seems like a bad idea to me.

--- a/src/libFLAC/stream_decoder.c
+++ b/src/libFLAC/stream_decoder.c
@@ -2609,6 +2609,9 @@ FLAC__bool read_subframe_fixed_(FLAC__StreamDecoder *decoder, unsigned channel,
                        FLAC__ASSERT(0);
        }
 
+       if (decoder->private_->frame.header.blocksize < order)
+               return true;
+
        /* decode the subframe */
        if(do_full_decode) {
                memcpy(decoder->private_->output[channel], subframe->warmup, sizeof(FLAC__int32) * order);
@@ -2688,6 +2691,9 @@ FLAC__bool read_subframe_lpc_(FLAC__StreamDecoder *decoder, unsigned channel, un
                        FLAC__ASSERT(0);
        }
 
+       if (decoder->private_->frame.header.blocksize < order)
+               return true;
+
        /* decode the subframe */
        if(do_full_decode) {
                memcpy(decoder->private_->output[channel], subframe->warmup, sizeof(FLAC__int32) * order);

-- 
Miroslav Lichvar