On Fri, Dec 22, 2023 at 09:57:33PM -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. >
> Reported-by: Michael Niedermayer <mich...@niedermayer.cc>
The issue has been discovered by Zeng Yunxiang and Li Zeyuan. as mentioned in
the ticket
> Signed-off-by: Leo Izen <leo.i...@gmail.com>
> ---
> libavcodec/jpegxl_parser.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> index 006eb6b295..c9832e4393 100644
> --- a/libavcodec/jpegxl_parser.c
> +++ b/libavcodec/jpegxl_parser.c
> @@ -388,7 +388,6 @@ static int populate_distribution(GetBitContext *gb,
> JXLSymbolDistribution *dist,
>
> if (get_bits1(gb)) {
> /* simple code */
> - dist->alphabet_size = 256;
> if (get_bits1(gb)) {
> uint8_t v1 = jxl_u8(gb);
> uint8_t v2 = jxl_u8(gb);
> @@ -398,10 +397,12 @@ static int populate_distribution(GetBitContext *gb,
> JXLSymbolDistribution *dist,
> dist->freq[v2] = (1 << 12) - dist->freq[v1];
> if (!dist->freq[v1])
> dist->uniq_pos = v2;
> + dist->alphabet_size = 1 + FFMAX(v1, v2);
> } else {
> uint8_t x = jxl_u8(gb);
> dist->freq[x] = 1 << 12;
> dist->uniq_pos = x;
> + dist->alphabet_size= 1 + x;
> }
> return 0;
> }
> @@ -880,6 +881,8 @@ static int read_distribution_bundle(GetBitContext *gb,
> JXLEntropyDecoder *dec,
> ret = populate_distribution(gb, &bundle->dists[i],
> bundle->log_alphabet_size);
> if (ret < 0)
> return ret;
> + if (bundle->dists[i].alphabet_size > (1 <<
> bundle->log_alphabet_size))
> + return AVERROR_INVALIDDATA;
i think alphabet_size should be checked before it is stored in the struct
or at least before it is used.
ATM the value is unchecked and substantial processing is done with it
in populate_distribution() before this check
Also log_alphabet_size for use_prefix_code == 0 is limited to a max of 8
which limits alphabet_size to 256 in that codepath with the new check.
There are also various arrays that can be reduced in size when alphabet_size
is limited in this codepath. But thats for a different time and patch.
For now i think just moving the alphabet_size check, is fine
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
