[FFmpeg-devel] [PATCH] EA ADPCM R1, R2 and R3
pross at xvid.org
Tue Oct 23 10:15:09 CEST 2007
On Tue, Oct 23, 2007 at 12:50:06AM +0200, Michael Niedermayer wrote:
> On Mon, Oct 22, 2007 at 12:05:53AM +0200, Aurelien Jacobs wrote:
> > On Sat, 20 Oct 2007 20:11:58 +0200
> > Michael Niedermayer <michaelni at gmx.at> wrote:
> > > also this is not the most readable way to check this especially if more
> > > codecs get added ...
> > I agree. After a quick look, it seems this check is here to prevent
> > overflow of the status context var. As this patch increase the size
> > of status to 6, I think it's safe to simply check for channels > 6U.
> > I hope I'm right here ?
> no, quick look is not enough, you (or me) would have to check all adpcm
> codecs for channel related security issues, maybe there are none but i
> dont feel good with removing this check after a quick look
> also i dont think the codecs will work with more than 2 channels after
> this check is removed ...
none of the existing adpcm decoders are >2 channel aware. michael's original comment
concerned readability, so why not just expand the if statement to a switch.
case CODEC_ID_ADPCM_EA_XAS: /* WiP */
if(avctx->channels > 6U)
if(avctx->channels > 2U)
> > > completely missing checks for array bounds, this should be exploitable
> > Right. Check added at the start of this decoding code.
> have you considered that the *channels in this check can overflow and thus
> the check would fail ...
that would be caught earlier by adpcm_decode_init using the "if channels>6U fail" sanity check.
More information about the ffmpeg-devel