Hi all,
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
CVE-2014-8962 : Heap buffer read overflow
For Linux distributions, the specific fixes for these two CVEs are available
from Git here:
https://git.xiph.org/?p=flac.git;a=commit;h=fcf0ba06ae12ccd7c67cee3c8d948df15f946b85
https://git.xiph.org/?p=flac.git;a=commit;h=5b3033a2b355068c11fe637e14ac742d273f076e
and are simple enough that they should apply cleanly to the last official
release 1.3.0 and possibly even the previous one, 1.2.1.
A pre-release (version 1.3.1pre1) for the next version which includes these
fixes and more is available here:
http://downloads.xiph.org/releases/flac/beta/
A full release (version 1.3.1) will be available in the next couple of days.
Cheers,
Erik
--
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/
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=fcf0ba06ae12ccd7c67cee3c8d948df15f946b85I'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
On Tue, Nov 25, 2014 at 12:29:33AM -0800, mle+la at mega-nerd.com wrote:> > CVE-2014-9028 : Heap buffer write overflow > CVE-2014-8962 : Heap buffer read overflowIs it known what other FLAC decoding software or firmware is vulnerable to these overflows? Any software player that was derived from the official FLAC codebase probably is, and most active 3rd party developers will probably get a new release out soon anyway, even if their code was not vulnerable. Embedded systems with native FLAC playback, such as DVD players and portable devices, may never get updated. -- -Dec. --- (no microsoft products were used to create this message) "Mosaic is going to be on every computer in the world." - Marc Andreessen, 1994
Erik de Castro Lopo wrote:> For Linux distributions, the specific fixes for these two CVEs are available > from Git here: > > https://git.xiph.org/?p=flac.git;a=commit;h=fcf0ba06ae12ccd7c67cee3c8d948df15f946b85A comment in the code about the patch: "We have received a potentially malicious bt stream" What "bt stream" means? or is it a typo (and it is "bit stream")? Also, in AUTHORS file: "Website : https://www.xip.org/flac/" xip -> xiph. So it's "https://www.xiph.org/flac/" (or maybe "http://www.xiph.org/flac/" or "http://xiph.org/flac/")
Miroslav Lichvar wrote:> 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?Yes.> 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?That may well be true. Is it possible to generate file that actually triggers this? Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
On Nov 25, 2014, at 8:27 AM, Declan Kelly <flac-dev at groov.ie> wrote:> > On Tue, Nov 25, 2014 at 12:29:33AM -0800, mle+la at mega-nerd.com wrote: >> >> CVE-2014-9028 : Heap buffer write overflow >> CVE-2014-8962 : Heap buffer read overflow > > Is it known what other FLAC decoding software or firmware is vulnerable > to these overflows? > > Any software player that was derived from the official FLAC codebase > probably is, and most active 3rd party developers will probably get a > new release out soon anyway, even if their code was not vulnerable.My impression - which may be out of date - is that many software players embedded the FLAC command line within their GUI app, and then never updated. This was a serious pain when FLAC was actively changing, gaining significant new features, and yet the most popular GUI apps were left behind. Some people were even left with a bad impression of FLAC because they never tried the command line, and thus were stuck with the buggy old versions in their GUI. You are correct that it is easier for a software player to be updated, but I have my doubts that any will be updated.> Embedded systems with native FLAC playback, such as DVD players and > portable devices, may never get updated.You might be surprised. If the device allows for firmware updates in the field, then the manufacturer may update their device before your favorite Windows GUI FLAC player. I have the Sound Devices 700 series field recorder that records directly to FLAC, and they've updated the firmware at least 6 or 7 times since I bought the device. I don't think more than one of the updates had anything to do with FLAC, but I don't feel worried that they're unable to correct a vulnerability. Granted, consumer DVD players might not get a firmware update, but these days even car stereos have a way for users to get the latest updates. Players like the OPPO surely have firmware update capabilities, if needed. While we're on the topic, what sort of consequences are there, really, with this vulnerability? Worst case, your player stops playing on a file that cannot be played anyway. Yes, it's bad that you have to power-cycle the player to get it to restart, but it's not like you can be doing anything else at the same time you're playing a bad FLAC. Have I missed something? Brian
lvqcl wrote:> Erik de Castro Lopo wrote: > > > For Linux distributions, the specific fixes for these two CVEs are available > > from Git here: > > > > https://git.xiph.org/?p=flac.git;a=commit;h=fcf0ba06ae12ccd7c67cee3c8d948df15f946b85 > > A comment in the code about the patch: > "We have received a potentially malicious bt stream" > What "bt stream" means? or is it a typo (and it is "bit stream")? > > > > Also, in AUTHORS file: > "Website : https://www.xip.org/flac/" > xip -> xiph. > So it's "https://www.xiph.org/flac/" > (or maybe "http://www.xiph.org/flac/" or "http://xiph.org/flac/")Thanks! Fixed. Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
On 25.11.2014 12:14, Miroslav Lichvar wrote:> 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?This patch breaks seeking in some perfectly valid files. So far I have received one sample full CD image from a foobar2000 user where a track is rendered inaccessible because of this. Re-encoding the file with FLAC 1.2.1 - 1.3.1 with identical settings doesn't remove the seeking problem. Either this patch needs to go or it needs to be altered to not prevent seek sync.
On 9.12.2014 20:33, Tristan Matthews wrote:> On Tue, Dec 9, 2014 at 1:31 PM, Janne Hyv?rinen <cse at sci.fi > <mailto:cse at sci.fi>> wrote: > > On 25.11.2014 12:14, Miroslav Lichvar wrote: > > 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? > > This patch breaks seeking in some perfectly valid files. So far I have > received one sample full CD image from a foobar2000 user where a track > is rendered inaccessible because of this. Re-encoding the file > with FLAC > 1.2.1 - 1.3.1 with identical settings doesn't remove the seeking > problem. > Either this patch needs to go or it needs to be altered to not prevent > seek sync. > > > Can you share samples?It's a 470 MB copyrighted music album. I could but I don't think it's legal. -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.xiph.org/pipermail/flac-dev/attachments/20141209/1a7aa03e/attachment.htm
Janne Hyv?rinen wrote:>> Can you share samples? > > It's a 470 MB copyrighted music album. I could but I don't think it's legal.How about sharing the album name (if it's popular enough)? (but then it would be good to post its CUETools verification log if the album has multiple pressings with different offsets...)