On Sun, Jan 03, 2016 at 07:50:39PM +0100, Andreas Cadhalpun wrote:
> 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
>
> vorbisdec.c | 5 +++++
> 1 file changed, 5 insertions(+)
> ba151dadb72b6c74e1139decf4b32c8676ddc58e
> 0001-vorbisdec-reject-rangebits-0.patch
> 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;
> + }
this assumes partitions > 0
iam not sure if this is required or not, the spec does not seem to
explicitly state it
> rangemax = (1 << rangebits);
> if (rangemax > vc->blocksize[1] / 2) {
> av_log(vc->avctx, AV_LOG_ERROR,
> --
> 2.6.4
>
> vorbisdec.c | 5 +++++
> 1 file changed, 5 insertions(+)
> d8862de6632822fd066a585953e5966fd941ad93
> 0002-vorbisdec-reject-channel-mapping-with-less-than-two-.patch
> 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.
LGTM
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
