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

[matthieu castet]

Jai Menon wrote:
> Hi,
> 
>> Hi,
>>
>> Jai Menon wrote:
>>> Hi,
>>>> check readsamplesize in alac, with the current code, it could be 
>>>> negative or bigger than 17 (and get_bits will fail).
>>>>
>>> And what about 24-bit alac streams? not to mention multi-channel alac if
>> possible
>>> that is...
>> ATM ffmpeg decoder doesn't support 24-bit stream, nor multi-channel :
>> - get_bits will return garbage if readsamplesize > 17
> 
> So isn't it better if we just check if channels>2 or wasted_bytes is nonzero
if channels = 2, wasted_bytes = 0, alac->setinfo_sample_size = 32
I got readsamplesize = 33. get_bits will fail, extend_sign32 will fail


> 
>> - in the 24-bit stream I saw on mplayer sample, wasted_bytes is used, so 
>> readsamplesize could be less than 17, but our decoder doesn't handle it 
>> correctly
>> - for not 16-bit stream, we got 'FIXME: unimplemented sample size'.
>> - MAX_CHANNELS is 2
>>
>> So until we implement correct support for multi-channel and other bits 
>> per sample, I think this check is safe.
>>
> 
> Hmm..couldn't we just add a check to see if the stream is multichannel 
Already done.

> 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"



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...