[MPlayer-dev-eng] Too big buffer in "libmpcodecs/ad_pcm.c"?

[Uoti Urpala]

On Mon, 2006-08-14 at 13:02 +0200, Reimar Döffinger wrote:
> > I'd either 
> > - just return an error if the minimum size exceeds maxlen (ridiculous
> >   number of channels), or
> 
> Are you really sure that audio_out->get_space() can't be larger than
> sh_audio->a_out_buffer_size?

Nothing prevents aos from returning huge values (though it could cause
some problems). However such an assumption isn't implied by what I said
above. It's enough to assume that maxlen >= minlen + audio_out_minsize,
and that should be guaranteed by the calling functions.
audio_out_minsize for ad_pcm is currently set to 2048, which means 512
channels with samplesize 4.

>  This really seems like a non-trivial
> assumption, and I don't really agree that simplifying
> maxlen = maxlen - maxlen % align;
> to
> return -1;
> is worth making such an assumption.

I think allowing return of partial data with some channels missing is
more of a bug than a feature. It'll just make the problems triggered in
filters and aos harder to diagnose.

> > - increase audio_out_minsize based on the number of channels in preinit
> >   (if you do that then no maxlen checks are needed in decode_audio).
> 
> which makes assumptions on how maxlen and minlen are created and as soon
> as somebody changes the way they are set in mplayer.c or mencoder.c we
> will have another bunch of security problems.
> Yes, obviously doing so will be a bug anyway, but I dislike programming
> in a way that makes bugs immediately security-relevant.

There already are decoders which rely on that assumption, and I think
they shouldn't all need extra checks. They're all called through
dec_audio.c, having the check there is better. In fact there already is
a check but it doesn't quite match what is actually required, I'll fix
it.