[flac-dev] Two new CVEs against FLAC

[Miroslav Lichvar]

On Tue, Nov 25, 2014 at 12:29:33AM -0800, Erik de Castro Lopo wrote:
> Google Security Team member, Michele Spagnuolo, recently found two potential
> problems in the FLAC code base. They are :
> 
> 
>     CVE-2014-9028 : Heap buffer write overflow

>     https://git.xiph.org/?p=flac.git;a=commit;h=fcf0ba06ae12ccd7c67cee3c8d948df15f946b85

I'm trying to figure out how this one works. It seems the problem is
integer underflow in the "frame.header.blocksize-order" expression
used in read_subframe_fixed_() and read_subframe_lpc_() to get the
number of encoded samples, which causes a buffer overflow in the
LPC/fixed subframe decoding.

The fix prevents that by returning false from
read_residual_partitioned_rice_() to stop further decoding of
the subframe when the partition order is 0 and blocksize is smaller
than the predictor order.

Is that correct?

I think the case with non-zero partition order may need to be fixed
too. For example, with partition order of 1, predictor order of 16 and
blocksize of 4, the function would return true and blocksize-order in
the caller would still underflow.

--- a/src/libFLAC/stream_decoder.c
+++ b/src/libFLAC/stream_decoder.c
@@ -2744,7 +2744,7 @@ FLAC__bool read_residual_partitioned_rice_(FLAC__StreamDecoder *decoder, unsigne
                if(partition_samples < predictor_order) {
                        send_error_to_client_(decoder, FLAC__STREAM_DECODER_ERROR_STATUS_LOST_SYNC);
                        decoder->protected_->state = FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC;
-                       return true;
+                       return false;
                }
        }

Thoughts?

-- 
Miroslav Lichvar