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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2352 bytes --]

On Tue, Dec 26, 2023 at 08:23:35AM -0600, Leo Izen wrote:
> 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@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.

it belongs in a seperate patch

also the 258 -> 256 should be a seperate patch IMHO
Its not part of fixing the ticket


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

yes

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".