Diego Biurrun <[email protected]> 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'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.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
