On 03.01.2016 02:41, Michael Niedermayer wrote:
> On Sun, Jan 03, 2016 at 01:36:13AM +0100, Andreas Cadhalpun wrote:
>> get_bits is documented to only support reading 1-25 bits.
>> get_bitsz was added for this purpose.
>>
>> Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
>> ---
>> libavcodec/vorbisdec.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
>> index f773afa..9ba0bce 100644
>> --- a/libavcodec/vorbisdec.c
>> +++ b/libavcodec/vorbisdec.c
>> @@ -169,7 +169,7 @@ static const char idx_err_str[] = "Index value %d out of
>> range (0 - %d) for %s a
>> }
>> #define GET_VALIDATED_INDEX(idx, bits, limit) \
>> {\
>> - idx = get_bits(gb, bits);\
>> + idx = get_bitsz(gb, bits);\
>> VALIDATE_INDEX(idx, limit)\
>> }
>
> this looks wrong
> bits is 8 or ilog() where ilog(0) is not allowed one way or another
> i think
>
> for example for the audio channels
> "the numbers read in the above two steps are channel numbers representing
> the channel to treat as magnitude and the channel to treat as angle,
> respectively. If for any coupling step the angle channel number equals
> the magnitude channel number " ... "the stream is undecodable."
>
> when reading 0 bits both would be 0
>
>
>>
>> @@ -585,7 +585,7 @@ static int vorbis_parse_setup_hdr_floors(vorbis_context
>> *vc)
>>
>> for (j = 0; j < floor_setup->data.t1.partitions; ++j) {
>> for (k = 0; k <
>> floor_setup->data.t1.class_dimensions[floor_setup->data.t1.partition_class[j]];
>> ++k, ++floor1_values) {
>> - floor_setup->data.t1.list[floor1_values].x =
>> get_bits(gb, rangebits);
>> + floor_setup->data.t1.list[floor1_values].x =
>> get_bitsz(gb, rangebits);
>
> IIUC
> "Vector [floor1_x_list] is limited to a maximum length of 65 elements;
> a setup indicating more than 65 total elements (including elements 0
> and 1 set prior to the read loop) renders the stream undecodable. All
> vector [floor1_x_list] element values must be unique within the vector;
> a non-unique value renders the stream undecodable. "
>
> suggests that values cannot be 0 as the first is hardcoded to 0
Thanks a lot for finding the relevant places in the specification.
Based on that it's probably better to just error out in these cases.
Patches for that are attached.
Best regards,
Andreas
>From d740a59b6e099c90504d55c65923def1ad6de234 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Sun, 3 Jan 2016 19:11:24 +0100
Subject: [PATCH 1/2] vorbisdec: reject rangebits 0
This causes non-unique elements in floor_setup->data.t1.list, which
makes the stream undecodable according to the specification.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/vorbisdec.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index f773afa..2792af1 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -573,6 +573,11 @@ static int vorbis_parse_setup_hdr_floors(vorbis_context *vc)
return AVERROR(ENOMEM);
rangebits = get_bits(gb, 4);
+ if (!rangebits) {
+ av_log(vc->avctx, AV_LOG_ERROR,
+ "A rangebits value of 0 is not compliant with the Vorbis I specification.\n");
+ return AVERROR_INVALIDDATA;
+ }
rangemax = (1 << rangebits);
if (rangemax > vc->blocksize[1] / 2) {
av_log(vc->avctx, AV_LOG_ERROR,
--
2.6.4
>From 09bf8ca4d496518b876639b793cae12066b7ba3b Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Sun, 3 Jan 2016 19:20:54 +0100
Subject: [PATCH 2/2] vorbisdec: reject channel mapping with less than two
channels
It causes the angle channel number to equal the magnitude channel
number, which makes the stream undecodable according to the
specification.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/vorbisdec.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libavcodec/vorbisdec.c b/libavcodec/vorbisdec.c
index 2792af1..313b637 100644
--- a/libavcodec/vorbisdec.c
+++ b/libavcodec/vorbisdec.c
@@ -794,6 +794,11 @@ static int vorbis_parse_setup_hdr_mappings(vorbis_context *vc)
if (get_bits1(gb)) {
mapping_setup->coupling_steps = get_bits(gb, 8) + 1;
+ if (vc->audio_channels < 2) {
+ av_log(vc->avctx, AV_LOG_ERROR,
+ "Square polar channel mapping with less than two channels is not compliant with the Vorbis I specification.\n");
+ return AVERROR_INVALIDDATA;
+ }
mapping_setup->magnitude = av_mallocz(mapping_setup->coupling_steps *
sizeof(*mapping_setup->magnitude));
mapping_setup->angle = av_mallocz(mapping_setup->coupling_steps *
--
2.6.4
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
