On Wed, Mar 11, 2015 at 01:23:23PM +0100, Niels Möller wrote:
> Diego Biurrun <di...@biurrun.de> writes:
>
> > #define DCA_XLL_DMIX_NCOEFFS_MAX (18)
> >
> > typedef struct XllChSetSubHeader {
> > int channels; ///< number of channels in channel set,
> > at most 16
> > int downmix_coeffs[DCA_XLL_DMIX_NCOEFFS_MAX];
> >
> > This looks like it could overflow on the second iteration if channels is
> > 16 as the comment claims it could be ...
>
> I don't think so. Above the verious loops which fill the
> chset->downmix_coeffs array, there's this check:
>
> + if (chset->downmix_ncoeffs > DCA_XLL_DMIX_NCOEFFS_MAX) {
> + av_log(s->avctx, AV_LOG_WARNING,
> + "XLL: Skipping %d downmix coefficients, exceeding
> impl. limit %d\n",
> + chset->downmix_ncoeffs, DCA_XLL_DMIX_NCOEFFS_MAX);
> + skip_bits_long(&s->gb, 9 * chset->downmix_ncoeffs);
> + chset->downmix_ncoeffs = 0;
> + } else {
>
> So in the overflow case, it skips the corresponding data in the
> bitstream, the loops filling the array are never executed, and
> chset->downmix_ncoeffs is set to zero, to reflect the fact that no
> coefficients were read from the bitstream. Do you think this check is
> insufficient, and if so, how?
I don't see that saving our skin, what if chset->downmix_ncoeffs is equal
to DCA_XLL_DMIX_NCOEFFS_MAX? Why shouldn't it overflow on the second
iteration if chset->channels is 16?
> I'm a bit concerned about the *reading* of the array. I don't remember
> the details of how these coeffficients are used, but after a quick look
> at the code where they are read, it seems downmix_ncoeffs is not used,
> and in the error case that code will read garbage data from the buffer
> and beyond. Not sure if there's any useful way to ignore the downmix
> coefficients and go on decoding (which the current code attempts). Safer
> alternatives might be to either return a PATCH_WELCOME error from the
> above check, or allocate the array dynamically.
That's probably a good idea, care to prepare a patch? :)
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
