[FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: Use correct number of codes when init default VLCs

[Andreas Rheinhardt]

Commit bbc0d0c1fe2b7ecdc4367295594f084f85ad22f5 made the mjpeg decoder
use default Huffman tables when none are given, yet when initializing
the default Huffman tables, it did not use the correct number of entries
of the arrays used to initialize the tables, but instead it used the
biggest entry + 1 (as if it were a continuous array 0..biggest entry).
This worked because the ff_init_vlc_sparse() (and its predecessors)
always skipped entries with a length of zero and the length of the
corresponding elements was always initialized to zero with only the
sizes of the actually existing elements being set to a size > 0 lateron.

Yet since commit 1249698e1b424cff8e77e6a83cfdbc9d11e01aa7 this is no
longer so, as build_vlc() actually read the array containing the values
itself. This implies that the wrong length now leads to a read beyond
the end of the given array; this could lead to crashs (but usually
doesn't); it is detectable by ASAN* and this commit fixes it.

*: AddressSanitizer: global-buffer-overflow on address xy
...
xy is located 0 bytes to the right of global variable 'avpriv_mjpeg_val_ac_luminance'

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
Hard to believe that this wrong number has not been found earlier.
The code in question has been touched by about a dozen commits.

 libavcodec/mjpegdec.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 4128c47303..0a5ef110d1 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -96,27 +96,26 @@ static int init_default_huffman_tables(MJpegDecodeContext *s)
         int index;
         const uint8_t *bits;
         const uint8_t *values;
-        int codes;
         int length;
     } ht[] = {
         { 0, 0, avpriv_mjpeg_bits_dc_luminance,
-                avpriv_mjpeg_val_dc, 12, 12 },
+                avpriv_mjpeg_val_dc, 12 },
         { 0, 1, avpriv_mjpeg_bits_dc_chrominance,
-                avpriv_mjpeg_val_dc, 12, 12 },
+                avpriv_mjpeg_val_dc, 12 },
         { 1, 0, avpriv_mjpeg_bits_ac_luminance,
-                avpriv_mjpeg_val_ac_luminance,   251, 162 },
+                avpriv_mjpeg_val_ac_luminance,   162 },
         { 1, 1, avpriv_mjpeg_bits_ac_chrominance,
-                avpriv_mjpeg_val_ac_chrominance, 251, 162 },
+                avpriv_mjpeg_val_ac_chrominance, 162 },
         { 2, 0, avpriv_mjpeg_bits_ac_luminance,
-                avpriv_mjpeg_val_ac_luminance,   251, 162 },
+                avpriv_mjpeg_val_ac_luminance,   162 },
         { 2, 1, avpriv_mjpeg_bits_ac_chrominance,
-                avpriv_mjpeg_val_ac_chrominance, 251, 162 },
+                avpriv_mjpeg_val_ac_chrominance, 162 },
     };
     int i, ret;
 
     for (i = 0; i < FF_ARRAY_ELEMS(ht); i++) {
         ret = build_vlc(&s->vlcs[ht[i].class][ht[i].index],
-                        ht[i].bits, ht[i].values, ht[i].codes,
+                        ht[i].bits, ht[i].values, ht[i].length,
                         0, ht[i].class == 1);
         if (ret < 0)
             return ret;
-- 
2.25.1