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

[Michael Niedermayer]

On Thu, Nov 24, 2016 at 01:06:35AM +0100, Andreas Cadhalpun wrote:
> On 23.11.2016 03:07, Michael Niedermayer wrote:
> > On Mon, Nov 14, 2016 at 09:55:15PM +0100, Andreas Cadhalpun wrote:
> >>  libopusdec.c |    6 ++++++
> >>  1 file changed, 6 insertions(+)
> >> 0b663c14f4a6dae3e1da453239dbe429aef7886e  0001-libopusdec-default-to-stereo-for-invalid-number-of-c.patch
> >> From d33ded293d15e8ceab666bea834d436f3a225bcc Mon Sep 17 00:00:00 2001
> >> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> Date: Mon, 14 Nov 2016 21:41:45 +0100
> >> Subject: [PATCH] libopusdec: default to stereo for invalid number of channels
> >>
> >> This fixes an out-of-bounds read if avc->channels is 0.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavcodec/libopusdec.c | 6 ++++++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> >> index acc62f1..61f68ed 100644
> >> --- a/libavcodec/libopusdec.c
> >> +++ b/libavcodec/libopusdec.c
> >> @@ -47,6 +47,12 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
> >>      int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled;
> >>      uint8_t mapping_arr[8] = { 0, 1 }, *mapping;
> >>  
> >> +    if (avc->channels <= 0) {
> >> +        av_log(avc, AV_LOG_WARNING,
> >> +               "Invalid number of channels %d, defaulting to stereo\n", avc->channels);
> >> +        avc->channels = 2;
> >> +    }
> > 
> > This looks wrong
> > 
> > opusdec uses ff_opus_parse_extradata() to set the number of channels
> > from extradata.
> > 
> > The value provided by the demuxer if any should not matter
> 
> However, extradata does not necessarily exist and in that case ff_opus_parse_extradata
> defaults to stereo, unless the demuxer has set channels to 1.
> This can also be done in libopusdec, but channels can still be 0, if the channel count
> in extradata is 0, so the above default setting is needed regardless.
> 
> Attached is an updated patch.
> 
> Best regards,
> Andreas
> 

>  libopusdec.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> bc2908b04551bef5476493cb7bbf0df4979f92f6  0001-libopusdec-default-to-stereo-for-invalid-number-of-c.patch
> From 7bee9f96947c76e6581e9bfa5ce87c3c94a1565e Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Mon, 14 Nov 2016 21:41:45 +0100
> Subject: [PATCH] libopusdec: default to stereo for invalid number of channels
> 
> This fixes an out-of-bounds read if avc->channels is 0.

LGTM

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161124/937091c4/attachment.sig>