ryan/mozjpeg
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix buffer overrun in 12-bit prog Huffman encoder

Browse Source 
Regression introduced by 16bd984557 and
5b177b3cab

The pre-computed absolute values used in encode_mcu_AC_first() and
encode_mcu_AC_refine() were stored in a JCOEF (signed short) array.
When attempting to losslessly transform a specially-crafted malformed
12-bit JPEG image with a coefficient value of -32768 into a progressive
12-bit JPEG image, the progressive Huffman encoder attempted to store
the absolute value of -32768 in the JCOEF array, thus overflowing the
16-bit signed data type.  Therefore, at this point in the code:
8c5e78ce29/jcphuff.c (L889)
the absolute value was read as -32768, which caused the test at
8c5e78ce29/jcphuff.c (L896)
to fail, falling through to
8c5e78ce29/jcphuff.c (L908)
with an overly large value of r (46) that, when shifted left four
places, incremented, and passed to emit_symbol(), exceeded the maximum
index (255) for the derived code tables.  Fortunately, the buffer
overrun was fully contained within phuff_entropy_encoder, so the issue
did not generate a segfault or other user-visible errant behavior, but
it did cause a UBSan failure that was detected by OSS-Fuzz.

This commit introduces an unsigned JCOEF (UJCOEF) data type and uses it
to store the absolute values of DCT coefficients computed by the
AC_first_prepare() and AC_refine_prepare() methods.

Note that the changes to the Arm Neon progressive Huffman encoder
extensions cause signed 16-bit instructions to be replaced with
equivalent unsigned 16-bit instructions, so the changes should be
performance-neutral.

Based on:
bbf61c0382

Closes #628
This commit is contained in:
DRC
2022-11-15 17:01:17 -06:00
parent aa3dd0bd29
commit 78a36f6dc3
14 changed files with 166 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
jcphuff.c
View File

@@ -5,7 +5,7 @@
* Copyright (C) 1995-1997, Thomas G. Lane.
* libjpeg-turbo Modifications:
* Copyright (C) 2011, 2015, 2018, 2021-2022, D. R. Commander.
* Copyright (C) 2016, 2018, Matthieu Darbois.
* Copyright (C) 2016, 2018, 2022, Matthieu Darbois.
* Copyright (C) 2020, Arm Limited.
* Copyright (C) 2021, Alex Richardson.
* For conditions of distribution and use, see the accompanying README.ijg
@@ -82,11 +82,11 @@ typedef struct {
/* Pointer to routine to prepare data for encode_mcu_AC_first() */
void (*AC_first_prepare) (const JCOEF *block,
const int *jpeg_natural_order_start, int Sl,
int Al, JCOEF *values, size_t *zerobits);
int Al, UJCOEF *values, size_t *zerobits);
/* Pointer to routine to prepare data for encode_mcu_AC_refine() */
int (*AC_refine_prepare) (const JCOEF *block,
const int *jpeg_natural_order_start, int Sl,
int Al, JCOEF *absvalues, size_t *bits);
int Al, UJCOEF *absvalues, size_t *bits);
/* Mode flag: TRUE for optimization, FALSE for actual data output */
boolean gather_statistics;
@@ -156,14 +156,14 @@ METHODDEF(boolean) encode_mcu_DC_first(j_compress_ptr cinfo,
JBLOCKROW *MCU_data);
METHODDEF(void) encode_mcu_AC_first_prepare
(const JCOEF *block, const int *jpeg_natural_order_start, int Sl, int Al,
JCOEF *values, size_t *zerobits);
UJCOEF *values, size_t *zerobits);
METHODDEF(boolean) encode_mcu_AC_first(j_compress_ptr cinfo,
JBLOCKROW *MCU_data);
METHODDEF(boolean) encode_mcu_DC_refine(j_compress_ptr cinfo,
JBLOCKROW *MCU_data);
METHODDEF(int) encode_mcu_AC_refine_prepare
(const JCOEF *block, const int *jpeg_natural_order_start, int Sl, int Al,
JCOEF *absvalues, size_t *bits);
UJCOEF *absvalues, size_t *bits);
METHODDEF(boolean) encode_mcu_AC_refine(j_compress_ptr cinfo,
JBLOCKROW *MCU_data);
METHODDEF(void) finish_pass_phuff(j_compress_ptr cinfo);
@@ -583,8 +583,8 @@ encode_mcu_DC_first(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
continue; \
/* For a negative coef, want temp2 = bitwise complement of abs(coef) */ \
temp2 ^= temp; \
values[k] = (JCOEF)temp; \
values[k + DCTSIZE2] = (JCOEF)temp2; \
values[k] = (UJCOEF)temp; \
values[k + DCTSIZE2] = (UJCOEF)temp2; \
zerobits |= ((size_t)1U) << k; \
} \
}
@@ -592,7 +592,7 @@ encode_mcu_DC_first(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
METHODDEF(void)
encode_mcu_AC_first_prepare(const JCOEF *block,
const int *jpeg_natural_order_start, int Sl,
int Al, JCOEF *values, size_t *bits)
int Al, UJCOEF *values, size_t *bits)
{
register int k, temp, temp2;
size_t zerobits = 0U;
@@ -665,9 +665,9 @@ encode_mcu_AC_first(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
register int nbits, r;
int Sl = cinfo->Se - cinfo->Ss + 1;
int Al = cinfo->Al;
JCOEF values_unaligned[2 * DCTSIZE2 + 15];
JCOEF *values;
const JCOEF *cvalue;
UJCOEF values_unaligned[2 * DCTSIZE2 + 15];
UJCOEF *values;
const UJCOEF *cvalue;
size_t zerobits;
size_t bits[8 / SIZEOF_SIZE_T];
@@ -680,7 +680,7 @@ encode_mcu_AC_first(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
emit_restart(entropy, entropy->next_restart_num);
#ifdef WITH_SIMD
cvalue = values = (JCOEF *)PAD((JUINTPTR)values_unaligned, 16);
cvalue = values = (UJCOEF *)PAD((JUINTPTR)values_unaligned, 16);
#else
/* Not using SIMD, so alignment is not needed */
cvalue = values = values_unaligned;
@@ -814,7 +814,7 @@ encode_mcu_DC_refine(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
zerobits |= ((size_t)1U) << k; \
signbits |= ((size_t)(temp2 + 1)) << k; \
} \
absvalues[k] = (JCOEF)temp; /* save abs value for main pass */ \
absvalues[k] = (UJCOEF)temp; /* save abs value for main pass */ \
if (temp == 1) \
EOB = k + koffset; /* EOB = index of last newly-nonzero coef */ \
} \
@@ -823,7 +823,7 @@ encode_mcu_DC_refine(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
METHODDEF(int)
encode_mcu_AC_refine_prepare(const JCOEF *block,
const int *jpeg_natural_order_start, int Sl,
int Al, JCOEF *absvalues, size_t *bits)
int Al, UJCOEF *absvalues, size_t *bits)
{
register int k, temp, temp2;
int EOB = 0;
@@ -930,9 +930,9 @@ encode_mcu_AC_refine(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
unsigned int BR;
int Sl = cinfo->Se - cinfo->Ss + 1;
int Al = cinfo->Al;
JCOEF absvalues_unaligned[DCTSIZE2 + 15];
JCOEF *absvalues;
const JCOEF *cabsvalue, *EOBPTR;
UJCOEF absvalues_unaligned[DCTSIZE2 + 15];
UJCOEF *absvalues;
const UJCOEF *cabsvalue, *EOBPTR;
size_t zerobits, signbits;
size_t bits[16 / SIZEOF_SIZE_T];
@@ -945,7 +945,7 @@ encode_mcu_AC_refine(j_compress_ptr cinfo, JBLOCKROW *MCU_data)
emit_restart(entropy, entropy->next_restart_num);
#ifdef WITH_SIMD
cabsvalue = absvalues = (JCOEF *)PAD((JUINTPTR)absvalues_unaligned, 16);
cabsvalue = absvalues = (UJCOEF *)PAD((JUINTPTR)absvalues_unaligned, 16);
#else
/* Not using SIMD, so alignment is not needed */
cabsvalue = absvalues = absvalues_unaligned;