[FFmpeg-devel] [PATCH] avcodec/jpegxl_parser: clear window

[Kacper Michajlow]

On Tue, 5 Nov 2024 at 11:05, Leo Izen <leo.izen at gmail.com> wrote:
>
> On 11/4/24 9:59 PM, James Almer wrote:
> > On 11/4/2024 11:23 PM, Michael Niedermayer wrote:
> >> Fixes: Use of uninitialised value of size 8
> >
> > Odd, the only load op with dec->window appears to be 4 bytes.
> >
> >> Fixes: 368725676/clusterfuzz-testcase-minimized-
> >> fuzzer_protocol_file-6022251122589696-cut
> >> Fixes: 42537758/clusterfuzz-testcase-minimized-
> >> fuzzer_protocol_file-5818969469026304-cut
> >>
> >> Found-by: ossfuzz
> >> Reported-by: Kacper Michajlow
> >> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >> ---
> >>   libavcodec/jpegxl_parser.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
> >> index 8c45e1a1b73..179ca1170bd 100644
> >> --- a/libavcodec/jpegxl_parser.c
> >> +++ b/libavcodec/jpegxl_parser.c
> >> @@ -847,7 +847,7 @@ static int read_distribution_bundle(GetBitContext
> >> *gb, JXLEntropyDecoder *dec,
> >>       }
> >>       if (bundle->lz77_enabled && !dec->window) {
> >> -        dec->window = av_malloc_array(1 << 20, sizeof(uint32_t));
> >> +        dec->window = calloc(1 << 20, sizeof(uint32_t));
> >
> > av_calloc(). How did it not crash for you once it was freed by av_free?
> >
> > Also, wouldn't this be hiding a bug? Reading memory that was expected to
> > be previously set.
> >
>
> It would be, as this is just a sliding LZ77 window. Reading
> uninitialized bytes that weren't written is a bug. I'd be curious to see
> the fuzz case so I could run it through valgrind and figure out why
> that's happening.
>
> This says reported by kacper, could you send me the fuzz testcase?

I've sent you testcases privately.

- Kacper