On Sun, Mar 08, 2015 at 03:27:52PM +0100, Niels Möller wrote: > Anton Khirnov <an...@khirnov.net> writes: > >> + return AVERROR_INVALIDDATA; > >> + } else { > >> + unsigned c, r; > >> + for (c = i = 0; c < s->xll_channels; c++, i += chset->channels + 1) { > >> + if ((chset->downmix_coeffs[i] = dca_get_inv_dmix_coeff(s)) == -1) > >> + return AVERROR_INVALIDDATA; > >> + for (r = 1; r <= chset->channels; r++) { > >> + int32_t coeff = dca_get_dmix_coeff(s); > >> + if (coeff == -1) > >> + return AVERROR_INVALIDDATA; > >> + chset->downmix_coeffs[i + r] = > >> + (chset->downmix_coeffs[i] * (int64_t) coeff + (1 << 15)) >> 16; > > > > Maybe I'm just missing something, but seems to me this can overflow the > > array. > > This code should be in a branch executed only if chset->downmix_ncoeffs > <= DCA_XLL_DMIX_NCOEFFS_MAX, which is an arbitrary limit. And the > intention is that chset->downmix_ncoeffs should equal the number of > coeffients read into the array. > > In the case that primary_ch_set is false, downmix_ncoeffs is set to > (chset->channels + 1) * s->xll_channels, which I think matches the loop > logic. > > It might make sense to add an assert (or whatever you prefer instead of > asserts) to check that the indices indeed are < > DCA_XLL_DMIX_NCOEFFS_MAX.
#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 ... Diego _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel