[FFmpeg-devel] [PATCH] alac : check readsamplesize

[Justin Ruggles]

Jai Menon wrote:
> Hi,
> 
> -------- Original-Nachricht --------
>> Datum: Mon, 14 Jul 2008 11:34:12 +0200
>> Von: matthieu castet <castet.matthieu at free.fr>
>> An: FFmpeg development discussions and patches <ffmpeg-devel at mplayerhq.hu>
>> Betreff: Re: [FFmpeg-devel] [PATCH] alac : check readsamplesize
> 
>> Jai Menon wrote:
>> [...]
>>> or > 16 bits sample size 
>> Done only at the end of decoding
>>
>>> and return an error from decode_frame instead of checking each value
>>> explicitly? 
>> What do you mean by "instead of checking each value explicitly"
>>
> 
> What I meant is to ask is wouldn't it be better if we could just recognise invalid files which
> quicktime/itunes couldn't possibly have generated, and die with an error from the decoder...

Maybe I'm misunderstanding you...  The decoder shouldn't check against
the Apple encoder limitations.  Since there is no ALAC "specification",
I think the overriding criterion for this should be whether the decoder
handles the value without doing anything wrong (crash, exploit, bad
output, etc...) even if we don't have a sample.

>> PS : note that with the current code channels = 1, wasted_bytes = 3, 
>> alac->setinfo_sample_size = 16, will give readsamplesize = -8.
>> readsamplesize is only use for reading, so it should be exploitable, but 
>> this will give some segdefault instead of clean failure...
> 
> Yes, and clean failure is good. 
> My point is just that maybe we could actually look into the 
> possibility of some combination of "parameters" which are totally invalid. These could be
> rejected outright and then we don't need to make redundant checks on valid samples.
> 
> Again, my interest here is purely to see if any of these sanity checks make sense from
> the encoder's perspective, which I'm in the process of writing as part od soc.

Checking the readsamplesize is basically the same thing as checking for
that invalid combination of parameters, but gets directly to the real
reason the combination is invalid.

-Justin