diff mbox series

[FFmpeg-devel] avcodec/jpegxl_parser: add some icc profile checks

Message ID 20231003173825.4357-1-leo.izen@gmail.com
State Accepted
Commit ec74553205a4e52598f9799d03fb2682f7d23831
Headers show
Series [FFmpeg-devel] avcodec/jpegxl_parser: add some icc profile checks | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

Leo Izen Oct. 3, 2023, 5:38 p.m. UTC 
This patch will cause the parser to abort if it detects an icc profile
with an invalid size. This is particularly important if the icc profile
is entropy-encoded with zero bits per symbol, as it can prevent a
seemingly infinite loop during parsing.

Fixes: infinite loop
Fixes: 62374/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer
    -5551878085410816

Found-by: continuous fuzzing process
    https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Reported-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Leo Izen <leo.izen@gmail.com>
---
 libavcodec/jpegxl_parser.c | 44 ++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

Comments

Leo Izen Oct. 4, 2023, 10:14 p.m. UTC | #1 
On 10/3/23 13:38, Leo Izen wrote:
> This patch will cause the parser to abort if it detects an icc profile
> with an invalid size. This is particularly important if the icc profile
> is entropy-encoded with zero bits per symbol, as it can prevent a
> seemingly infinite loop during parsing.
> 
> Fixes: infinite loop
> Fixes: 62374/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer
>      -5551878085410816
> 
> Found-by: continuous fuzzing process
>      https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Reported-by: Michael Niedermayer <michael@niedermayer.cc>
> Signed-off-by: Leo Izen <leo.izen@gmail.com>
> ---
>   libavcodec/jpegxl_parser.c | 44 ++++++++++++++++++++++++++++++--------
>   1 file changed, 35 insertions(+), 9 deletions(-)
> 


Will merge soon as it fixes a clusterfuzz case.

- Leo Izen
diff mbox series

Patch

diff --git a/libavcodec/jpegxl_parser.c b/libavcodec/jpegxl_parser.c
index d25a1b6e1d..bbd7338a61 100644
--- a/libavcodec/jpegxl_parser.c
+++ b/libavcodec/jpegxl_parser.c
@@ -1044,34 +1044,60 @@  static int skip_icc_profile(void *avctx, JXLParseContext *ctx, GetBitContext *gb
 {
     int64_t ret;
     uint32_t last = 0, last2 = 0;
-    JXLEntropyDecoder dec;
+    JXLEntropyDecoder dec = { 0 };
     uint64_t enc_size = jxl_u64(gb);
+    uint64_t output_size = 0;
+    int out_size_shift = 0;
 
-    if (!enc_size)
+    if (!enc_size || enc_size > (1 << 22))
         return AVERROR_INVALIDDATA;
 
     ret = entropy_decoder_init(avctx, gb, &dec, 41);
     if (ret < 0)
-        return ret;
+        goto end;
 
     if (get_bits_left(gb) < 0) {
-        entropy_decoder_close(&dec);
-        return AVERROR_BUFFER_TOO_SMALL;
+        ret = AVERROR_BUFFER_TOO_SMALL;
+        goto end;
     }
 
     for (uint64_t read = 0; read < enc_size; read++) {
         ret = entropy_decoder_read_symbol(gb, &dec, icc_context(read, last, last2));
-        if (ret < 0 || get_bits_left(gb) < 0) {
-            entropy_decoder_close(&dec);
-            return ret < 0 ? ret : AVERROR_BUFFER_TOO_SMALL;
+        if (ret < 0)
+            goto end;
+        if (ret > 255) {
+            ret = AVERROR_INVALIDDATA;
+            goto end;
+        }
+        if (get_bits_left(gb) < 0) {
+            ret = AVERROR_BUFFER_TOO_SMALL;
+            goto end;
         }
         last2 = last;
         last = ret;
+        if (out_size_shift < 63) {
+            output_size += (ret & UINT64_C(0x7F)) << out_size_shift;
+            if (!(ret & 0x80)) {
+                out_size_shift = 63;
+            } else {
+                out_size_shift += 7;
+                if (out_size_shift > 56) {
+                    ret = AVERROR_INVALIDDATA;
+                    goto end;
+                }
+            }
+        } else if (output_size < 132) {
+            ret = AVERROR_INVALIDDATA;
+            goto end;
+        }
     }
 
+    ret = 0;
+
+end:
     entropy_decoder_close(&dec);
 
-    return 0;
+    return ret;
 }
 
 static int skip_extensions(GetBitContext *gb)