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

[Leo Izen <leo.izen@gmail.com>]

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;
 
     /*
      * each (1 << log_alphabet_size) length
      * with log_alphabet_size <= 8
      */
     /* frequencies associated with this Distribution */
-    uint32_t freq[258];
+    uint32_t freq[256];
     /* cutoffs for using the symbol table */
-    uint16_t cutoffs[258];
+    uint16_t cutoffs[256];
     /* the symbol table for this distribution */
-    uint16_t symbols[258];
+    uint16_t symbols[256];
     /* the offset for symbols */
-    uint16_t offsets[258];
+    uint16_t offsets[256];
 
     /* if this distribution contains only one symbol this is its index */
     int uniq_pos;
@@ -382,13 +382,13 @@ static int populate_distribution(GetBitContext *gb, JXLSymbolDistribution *dist,
     int len = 0, shift, omit_log = -1, omit_pos = -1;
     int prev = 0, num_same = 0;
     uint32_t total_count = 0;
-    uint8_t logcounts[258] = { 0 };
-    uint8_t same[258] = { 0 };
+    uint8_t logcounts[256] = { 0 };
+    uint8_t same[256] = { 0 };
+    const int table_size = 1 << log_alphabet_size;
     dist->uniq_pos = -1;
 
     if (get_bits1(gb)) {
         /* simple code */
-        dist->alphabet_size = 256;
         if (get_bits1(gb)) {
             uint8_t v1 = jxl_u8(gb);
             uint8_t v2 = jxl_u8(gb);
@@ -398,17 +398,24 @@ static int populate_distribution(GetBitContext *gb, JXLSymbolDistribution *dist,
             dist->freq[v2] = (1 << 12) - dist->freq[v1];
             if (!dist->freq[v1])
                 dist->uniq_pos = v2;
+            dist->alphabet_size = 1 + FFMAX(v1, v2);
         } else {
             uint8_t x = jxl_u8(gb);
             dist->freq[x] = 1 << 12;
             dist->uniq_pos = x;
+            dist->alphabet_size = 1 + x;
         }
+        if (dist->alphabet_size > table_size)
+            return AVERROR_INVALIDDATA;
+
         return 0;
     }
 
     if (get_bits1(gb)) {
         /* flat code */
         dist->alphabet_size = jxl_u8(gb) + 1;
+        if (dist->alphabet_size > table_size)
+            return AVERROR_INVALIDDATA;
         for (int i = 0; i < dist->alphabet_size; i++)
             dist->freq[i] = (1 << 12) / dist->alphabet_size;
         for (int i = 0; i < (1 << 12) % dist->alphabet_size; i++)
@@ -426,6 +433,9 @@ static int populate_distribution(GetBitContext *gb, JXLSymbolDistribution *dist,
         return AVERROR_INVALIDDATA;
 
     dist->alphabet_size = jxl_u8(gb) + 3;
+    if (dist->alphabet_size > table_size)
+        return AVERROR_INVALIDDATA;
+
     for (int i = 0; i < dist->alphabet_size; i++) {
         logcounts[i] = get_vlc2(gb, dist_prefix_table, 7, 1);
         if (logcounts[i] == 13) {
-- 
2.43.0

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