[FFmpeg-devel] [PATCH v2] avcodec/jpegxl_parser: check ANS cluster alphabet size vs bundle size

[Leo Izen]

On 12/25/23 15:09, Michael Niedermayer wrote:
> On Mon, Dec 25, 2023 at 12:04:17PM -0500, Leo Izen wrote:
>> The specification doesn't mention that clusters cannot have alphabet
>> sizes greater than 1 << bundle->log_alphabet_size, but the reference
>> implementation rejects these entropy streams as invalid, so we should
>> too. Refusing to do so can overflow a stack variable on line 556 that
>> should be large enough otherwise.
>>
>> Fixes #10738.
>>
>> Found-by: Zeng Yunxiang and Li Zeyuan
>> Signed-off-by: Leo Izen <leo.izen at gmail.com>
>> ---
>>   libavcodec/jpegxl_parser.c | 28 +++++++++++++++++++---------
>>   1 file changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
>> index 006eb6b295..f026fda9ac 100644
>> --- a/libavcodec/jpegxl_parser.c
>> +++ b/libavcodec/jpegxl_parser.c
>> @@ -64,26 +64,26 @@ typedef struct JXLSymbolDistribution {
>>       int log_bucket_size;
>>       /* this is the actual size of the alphabet */
>>       int alphabet_size;
>> -    /* ceil(log(alphabet_size)) */
>> -    int log_alphabet_size;
>>   
>>       /* for prefix code distributions */
>>       VLC vlc;
>>       /* in case bits == 0 */
>>       uint32_t default_symbol;
>> +    /* ceil(log(alphabet_size)) */
>> +    int log_alphabet_size;
>>   
> 
> that seems unneeded
> 

dist->log_alphaebet_size is only used for prefix code distributions so I 
moved it for clarity. I can also remove this change from this commit if 
you think it's off-topic.

In either case, is the commit okay, apart from this one change? If so 
I'm going to merge it (after I remove this one change from the diff).

- Leo Izen (Traneptora)