PR #25511 opened by Marti2203
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25511
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25511.patch
`skip_boxes()` rejected two valid ISOBMFF box sizes as invalid: an empty box
(size equal to the header size, 8 or 16 bytes) and a box of size 0 ("extends to
the end of the file"). On `AVERROR_INVALIDDATA`, `jpegxl_parse()` set
`ctx->next = -1`. That turns off the image-boundary search for the rest of the
stream, so every following image was merged into one packet, which only came
out at EOF, and nothing was logged.
Changes:
- empty boxes are skipped like any other box;
- size 0 explicitly means "the rest of the stream belongs to this image". That
was already the effective behaviour, but it was reached through the
invalid-data path. Boxes too large to track (`> INT_MAX - skip`) are handled
the same way, as before;
- a box smaller than its header (`size < head_size`, including a 64-bit
largesize below 16) is really invalid. The parser now logs an error and ends
the current image at that box, instead of silently swallowing the rest of the
stream.
Example: two concatenated container images through `jpegxl_pipe`, with the
first one ending in an empty `free` box. Before the patch this gives one
152-byte packet. With the patch it gives two packets (80 + 72 bytes). On a
200-image stream with random empty, non-empty and largesize `free` boxes, the
parser outputs 1 packet before the patch and 197 after. The last 4 images are
merged at EOF in both cases; that is a separate, existing limitation, and plain
concatenated streams with no extra boxes show it too.
Testing: the existing `fate-jxl-anim-demux-*`, `fate-jxl-small-ext-box` and
`fate-jxl-multiframe-permuted-toc` tests pass unchanged.
For a FATE test I have a 152-byte sample, [concat-empty-box.jxl](SAMPLE_URL):
two copies of `belgium.jxl` in containers, the first ending in an empty `free`
box, the second with a size-0 `jxlc` box. Could someone with fate-suite access
upload it as `jxl/concat-empty-box.jxl`? I would then add this test (it fails
without the fix, one 152-byte packet, and passes with it, 80 + 72 bytes):
```diff
diff --git a/tests/fate/jxl.mak b/tests/fate/jxl.mak
index 1416b98f6d..3bb4ba7079 100644
--- a/tests/fate/jxl.mak
+++ b/tests/fate/jxl.mak
@@ -22,6 +22,10 @@ fate-jxl-small-ext-box: CMD = framecrc -i
$(TARGET_SAMPLES)/jxl/l.jxl -c copy
FATE_JPEGXL_PARSE += fate-jxl-multiframe-permuted-toc
fate-jxl-multiframe-permuted-toc: CMD = framecrc -i
$(TARGET_SAMPLES)/jxl/orange.jxl -c copy
+# two concatenated images, the first ends with an empty box, the last box of
the second has size 0
+FATE_JPEGXL_PARSE += fate-jxl-parse-empty-box
+fate-jxl-parse-empty-box: CMD = framecrc -f jpegxl_pipe -i
$(TARGET_SAMPLES)/jxl/concat-empty-box.jxl -c copy
+
FATE_JPEGXL_PARSE += $(FATE_JPEGXL_PARSE-yes)
FATE_SAMPLES_FFMPEG-$(call FRAMECRC, IMAGE_JPEGXL_PIPE, , JPEGXL_PARSER) +=
$(FATE_JPEGXL_PARSE)
fate-jxl-parse: $(FATE_JPEGXL_PARSE)
diff --git a/tests/ref/fate/jxl-parse-empty-box
b/tests/ref/fate/jxl-parse-empty-box
new file mode 100644
index 0000000000..c4390d724b
--- /dev/null
+++ b/tests/ref/fate/jxl-parse-empty-box
@@ -0,0 +1,7 @@
+#tb 0: 1/25
+#media_type 0: video
+#codec_id 0: jpegxl
+#dimensions 0: 768x512
+#sar 0: 0/1
+0, 0, 0, 1, 80, 0xd74d1418
+0, 1, 1, 1, 72, 0x39d01246
```
This issue was found by an LLM-Enhanced Static Analysis Engine and verified
manually.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
From 135fe5fcf48d6cc90bab0e45a4fad2beae63c720 Mon Sep 17 00:00:00 2001
From: Martin Mirchev <[email protected]>
Date: Tue, 27 Oct 2026 10:42:10 +0800
Subject: [PATCH] avcodec/jpegxl_parser: fix handling of special ISOBMFF box
sizes
skip_boxes() treated a box of size 0 and an empty box (size equal to
the header size) as invalid. jpegxl_parse() then set ctx->next to -1,
which disables the search for the end of the image for the rest of the
stream, so all following images were merged into a single packet,
which is only output at EOF.
An empty box is valid and is now skipped. A box of size 0 extends to
the end of the file, so the rest of the stream belongs to the current
image; keep that behaviour, but explicitly. Boxes too large to track
are treated the same way, as before. A box smaller than its header is
invalid: log an error and end the current image at that box instead
of silently swallowing the rest of the stream.
Signed-off-by: Martin Mirchev <[email protected]>
---
libavcodec/jpegxl_parser.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
index a16a2733f9d..902a1532aac 100644
--- a/libavcodec/jpegxl_parser.c
+++ b/libavcodec/jpegxl_parser.c
@@ -1393,12 +1393,15 @@ static int skip_boxes(JXLParseContext *ctx, const
uint8_t *buf, int buf_size)
return AVERROR_BUFFER_TOO_SMALL;
size = bytestream2_get_be64(&gb);
head_size = 16;
+ } else if (!size) {
+ /* the box extends to the end of the file */
+ return END_NOT_FOUND;
}
- if (!size)
- return AVERROR_INVALIDDATA;
/* invalid ISOBMFF size */
- if (size <= head_size || size > INT_MAX - ctx->skip)
+ if (size < head_size)
return AVERROR_INVALIDDATA;
+ if (size > INT_MAX - ctx->skip)
+ return END_NOT_FOUND;
ctx->skip += size;
bytestream2_skip(&gb, size - head_size);
@@ -1512,12 +1515,19 @@ static int jpegxl_parse(AVCodecParserContext *s,
AVCodecContext *avctx,
if (ctx->container && ctx->next >= 0) {
ret = skip_boxes(ctx, pbuf, pindex);
- if (ret < 0) {
- if (ret == AVERROR_INVALIDDATA)
- ctx->next = -1;
+ if (ret == END_NOT_FOUND) {
+ /* the rest of the stream belongs to this image */
+ ctx->next = -1;
goto flush;
+ } else if (ret == AVERROR_INVALIDDATA) {
+ /* end the image at the invalid box */
+ av_log(avctx, AV_LOG_ERROR, "Invalid ISOBMFF box size\n");
+ ctx->next = ctx->skip;
+ } else if (ret < 0) {
+ goto flush;
+ } else {
+ ctx->next = ret + ctx->skip;
}
- ctx->next = ret + ctx->skip;
}
if (ctx->next >= 0)
--
2.54.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
