Re: [FFmpeg-devel] [FFmpeg-cvslog] avformat/jpegxl_anim_dec: add animated JPEG XL demuxer

[Michael Niedermayer <michael@niedermayer.cc>]

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

On Mon, Jun 05, 2023 at 04:15:32PM +0000, Leo Izen wrote:
> ffmpeg | branch: master | Leo Izen <leo.izen@gmail.com> | Fri Mar  3 15:31:46 2023 -0500| [fa11c4c7fa3961b0101daaa0032bb26a7a1a9c0f] | committer: Leo Izen
> 
> avformat/jpegxl_anim_dec: add animated JPEG XL demuxer
> 
> Animated JPEG XL files requires a separate demuxer than image2, because
> the timebase information is set by the demuxer. Should the timebase of
> an animated JPEG XL file be incompatible with the timebase set by the
> image2pipe demuxer (usually 1/25 unless set otherwise), rescaling will
> fail. Adding a separate demuxer for animated JPEG XL files allows the
> timebase to be set correctly.
> 
> Signed-off-by: Leo Izen <leo.izen@gmail.com>
> 
> > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=fa11c4c7fa3961b0101daaa0032bb26a7a1a9c0f
> ---

[...]

> +/*
> + * copies as much of the codestream into the buffer as possible
> + * pass a shorter buflen to request less
> + * returns the number of bytes consumed from input, may be greater than input_len
> + * if the input doesn't end on an ISOBMFF-box boundary
> + */
> +static int jpegxl_collect_codestream_header(const uint8_t *input_buffer, int input_len, uint8_t *buffer, int buflen, int *copied) {
> +    const uint8_t *b = input_buffer;
> +    *copied = 0;
> +
> +    while (1) {
> +        uint64_t size;
> +        uint32_t tag;
> +        int head_size = 8;
> +
> +        if (b - input_buffer >= input_len - 16)
> +            break;
> +
> +        size = AV_RB32(b);
> +        b += 4;
> +        if (size == 1) {
> +            size = AV_RB64(b);
> +            b += 8;
> +            head_size = 16;
> +        }
> +        /* invalid ISOBMFF size */
> +        if (size > 0 && size <= head_size)
> +            return AVERROR_INVALIDDATA;
> +        if (size > 0)
> +            size -= head_size;
> +
> +        tag = AV_RL32(b);
> +        b += 4;
> +        if (tag == MKTAG('j', 'x', 'l', 'p')) {
> +            b += 4;
> +            size -= 4;
> +        }

This checks that there are 17 bytes but then advances by 20 (4+8+4+4)
It checks if uint64_t size is not 0 then even if 0 subtracts 4, 6 lines later
and a few lines later below adds size with no checks to a pointer
Pointers must stay valid, its undefined behavior to move a pointer beyond
the end of the allocated space, even if its not dereferenced.
Even if it wasnt undefined this would not be safe because it could go
over the end of addressspace

thx

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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

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