Guard against dupe SOF w/ incorrect source manager

Referring to https://bugzilla.mozilla.org/show_bug.cgi?id=1898606,
attempting to decompress a specially-crafted malformed JPEG image
(specifically an image with a complete 12-bit Start Of Frame segment
followed by an incomplete 8-bit Start Of Frame segment) using the
default marker processor, buffered-image mode, and input prefetching
triggered the following sequence of events:

- When the 12-bit SOF segment was encountered (in the body of
  jpeg_read_header()), the marker processor's read_markers() method
  called the get_sof() function, which processed the 12-bit SOF segment
  and set cinfo->data_precision to 12.

- If the application subsequently called jpeg_consume_input() in a loop
  to prefetch input data, and it didn't stop calling
  jpeg_consume_input() when the function returned JPEG_REACHED_SOS, then
  the 8-bit SOF segment was encountered in the body of
  jpeg_consume_input().  As a result, the marker processor's
  read_markers() method called get_sof(), which started to process the
  8-bit SOF segment and set cinfo->data_precision to 8.

- Since the 8-bit SOF segment was incomplete, the end of the JPEG data
  stream was encountered when get_sof() attempted to read the image
  height, width, and number of components.

- If the fill_input_buffer() method in the application's custom source
  manager incorrectly returned FALSE in response to a prematurely-
  terminated JPEG data stream, then get_sof() returned FALSE while
  attempting to read the image height, width, and number of components
  (before the duplicate SOF check was reached.)  That caused the default
  marker processor's read_markers() method, and subsequently
  jpeg_consume_input(), to return JPEG_SUSPENDED.

- If the application failed to respond to the JPEG_SUSPENDED return
  value and subsequently attempted to call jpeg_read_scanlines(),
  then the data precision check in jpeg_read_scanlines() succeeded
  (because cinfo->data_precision was now 8.)  However, because
  cinfo->data_precision had been 12 during the previous call to
  jpeg_start_decompress(), only the 12-bit version of the main
  controller was initialized, and the cinfo->main->process_data() method
  was undefined.  Thus, a segfault occurred when jpeg_read_scanlines()
  attempted to invoke that method.

Scenarios in which the issue was thwarted:

1. The default source managers handle a prematurely-terminated JPEG data
stream by inserting a fake EOI marker into the data stream.  Thus, when
using one of those source managers, the INPUT_2BYTES() and INPUT_BYTE()
macros (which get_sof() invokes to read the image height, width, and
number of components) succeeded-- albeit with bogus data, since the fake
EOI marker was read into those fields.  The duplicate SOF check in
get_sof() then failed, generating a fatal libjpeg error.

2. When using a custom source manager that correctly returns TRUE in
response to a prematurely-terminated JPEG data stream, the
aforementioned INPUT_2BYTES() and INPUT_BYTE() macros also succeeded
(albeit with bogus data read from the previous bytes of the data
stream), and the duplicate SOF check failed.

3. If the application did not prefetch input data, or if it stopped
invoking jpeg_consume_input() when the function returned
JPEG_REACHED_SOS, then the duplicate SOF segment was not read prior to
the first call to jpeg_read_scanlines().  Thus, the data precision check
in jpeg_read_scanlines() failed.  If the application instead called
jpeg12_read_scanlines() (that is, if it properly supported multiple data
precisions), then the duplicate SOF segment was not read until the body
of jpeg_finish_decompress().  At that point, its only negative effect
was to cause jpeg_finish_decompress() to return FALSE before the
duplicate SOF check was reached.

In other words, this issue depended not only upon an incorrectly-written
source manager but also upon a very specific sequence of API calls.  It
also depended upon the multi-precision feature introduced in
libjpeg-turbo 3.0.x.  When using an 8-bit-per-sample build of
libjpeg-turbo 2.1.x, jpeg_read_header() failed with "Unsupported JPEG
data precision 12" after the 12-bit SOF segment was processed.  When
using a 12-bit-per-sample build of libjpeg-turbo 2.1.x, the behavior
was the same as if the application called jpeg12_read_scanlines() in
Scenario 3 above.

This commit simply moves the duplicate SOF check to the top of
get_sof() so the check will fail before the marker processor attempts to
read the duplicate SOF.  It should be noted that this issue isn't a
libjpeg-turbo bug per se, because it occurs only when the calling
application does something it shouldn't.  It is, rather, an issue of API
hardening/caller-proofing.
This commit is contained in:
DRC
2024-05-27 11:41:35 -04:00
parent 46173635be
commit 3c17063ef1
2 changed files with 12 additions and 3 deletions

View File

@@ -10,6 +10,15 @@ unreasonable slow-down in `jpeg_read_header()` if an application called
to decompress a JPEG image containing an excessive number of markers of that to decompress a JPEG image containing an excessive number of markers of that
type. type.
2. Hardened the default marker processor in the decompressor to guard against
an issue (exposed by 3.0 beta2[6]) whereby attempting to decompress a
specially-crafted malformed JPEG image (specifically an image with a complete
12-bit-per-component Start Of Frame segment followed by an incomplete
8-bit-per-component Start Of Frame segment) using buffered-image mode and input
prefetching caused a segfault if the `fill_input_buffer()` method in the
calling application's custom source manager incorrectly returned `FALSE` in
response to a prematurely-terminated JPEG data stream.
3.0.3 3.0.3
===== =====

View File

@@ -248,6 +248,9 @@ get_sof(j_decompress_ptr cinfo, boolean is_prog, boolean is_lossless,
jpeg_component_info *compptr; jpeg_component_info *compptr;
INPUT_VARS(cinfo); INPUT_VARS(cinfo);
if (cinfo->marker->saw_SOF)
ERREXIT(cinfo, JERR_SOF_DUPLICATE);
cinfo->progressive_mode = is_prog; cinfo->progressive_mode = is_prog;
cinfo->master->lossless = is_lossless; cinfo->master->lossless = is_lossless;
cinfo->arith_code = is_arith; cinfo->arith_code = is_arith;
@@ -265,9 +268,6 @@ get_sof(j_decompress_ptr cinfo, boolean is_prog, boolean is_lossless,
(int)cinfo->image_width, (int)cinfo->image_height, (int)cinfo->image_width, (int)cinfo->image_height,
cinfo->num_components); cinfo->num_components);
if (cinfo->marker->saw_SOF)
ERREXIT(cinfo, JERR_SOF_DUPLICATE);
/* We don't support files in which the image height is initially specified */ /* We don't support files in which the image height is initially specified */
/* as 0 and is later redefined by DNL. As long as we have to check that, */ /* as 0 and is later redefined by DNL. As long as we have to check that, */
/* might as well have a general sanity check. */ /* might as well have a general sanity check. */