[FFmpeg-devel] [libav-devel] [PATCH] libopusdec: fix out-of-bounds read

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Mon Nov 14 22:55:15 EET 2016


On 14.11.2016 20:54, Anton Khirnov wrote:
> Quoting Andreas Cadhalpun (2016-11-14 20:30:10)
>> On 14.11.2016 00:01, Luca Barbato wrote:
>>> On 13/11/2016 19:23, Andreas Cadhalpun wrote:
>>>> avc->channels can be 0.
>>>
>>> 0 and less than zero shouldn't be an error?
>>
>> Such values should be rejected, wherever they are set.
>> However, ensuring that is a larger change I'm currently
>> working on.
>> Meanwhile, this patch is a trivial fix for the potential
>> security problem that can easily be backported.
> 
> channels being zero is perfectly valid, it means the caller does not
> know the channel count and expects the decoder to read it from the
> bitstream.

In general code this is correct, however if e.g. the matroska demuxer
reads an audio stream which claims to have 0 channels, it should
be rejected as broken.

> This should fail for codecs that do not store this
> information in the bitstream, but work fine otherwise.
> 
> In the case of opus, the channel count is always known -- when the
> extradata is present, the channel count is stored there. Otherwise the
> stream is simple and can be decoded either as mono or stereo, as we
> want.
> 
> The patch does not seem to be doing the right thing -- I think it will
> simply fail on the opus_multistream_decoder_create() call.

Correct.

> What it should do instead is just default to stereo.

OK, patch doing that is attached.

> Even better, you could replace the whole extradata parsing block with
> a call to ff_opus_parse_extradata(), though that would require some
> refactoring.

The fix should be easily backportable, which excludes refactoring.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libopusdec-default-to-stereo-for-invalid-number-of-c.patch
Type: text/x-diff
Size: 1179 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161114/9d4b03ce/attachment.patch>


More information about the ffmpeg-devel mailing list