| Message ID | 9fc253e0-cb6d-eb66-66c5-af6f7c37e903@googlemail.com |
|---|---|
| State | Accepted |
| Headers | show |
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@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@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@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 [...]
On 24.11.2016 16:38, Michael Niedermayer wrote: > On Thu, Nov 24, 2016 at 01:06:35AM +0100, Andreas Cadhalpun wrote: >> 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@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 Pushed. Best regards, Andreas
From 7bee9f96947c76e6581e9bfa5ce87c3c94a1565e Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun <Andreas.Cadhalpun@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@googlemail.com> --- libavcodec/libopusdec.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c index acc62f1..e6ca61a 100644 --- a/libavcodec/libopusdec.c +++ b/libavcodec/libopusdec.c @@ -47,6 +47,13 @@ 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; + avc->channels = avc->extradata_size >= 10 ? avc->extradata[9] : (avc->channels == 1) ? 1 : 2; + if (avc->channels <= 0) { + av_log(avc, AV_LOG_WARNING, + "Invalid number of channels %d, defaulting to stereo\n", avc->channels); + avc->channels = 2; + } + avc->sample_rate = 48000; avc->sample_fmt = avc->request_sample_fmt == AV_SAMPLE_FMT_FLT ? AV_SAMPLE_FMT_FLT : AV_SAMPLE_FMT_S16; -- 2.10.2