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

tj3Transform(): Ensure JPEG hdr successfully read

Browse Source 
Because of the TurboJPEG 3 API overhaul, the legacy decompression and
lossless transformation functions now wrap the new TurboJPEG 3
functions.  For performance reasons, we don't want to read the JPEG
header more than once during the same operation, so the wrapped
functions do not read the header if it has already been read by a
wrapper function.  Initially the TurboJPEG 3 functions used a state
variable to track whether the header had already been read, but
b94041390c made this more robust by using
the libjpeg global decompression state instead.  If a wrapper function
has already read the JPEG header successfully, then the global
decompression state will be DSTATE_READY, and the logic introduced in
b94041390c will prevent the header from
being read again.

A subtle issue arises because tj3DecompressHeader() does not call
jpeg_abort_decompress() if jpeg_read_header() fails.  (That is arguably
a bug, but it has existed since the very first implementation of the
function.)  Depending on the nature of the failure, this can cause
tj3DecompressHeader() to return an error code and leave the libjpeg
global decompression state set to DSTATE_INHEADER.  If a misbehaved
application ignored the error and subsequently called a TurboJPEG
decompression or lossless transformation function, then the function
would fail to read the JPEG header because the global decompression
state was greater than DSTATE_START.  In the case of the decompression
functions, this was innocuous, because jpeg_calc_output_dimensions()
and jpeg_start_decompress() both sanity check the global decompression
state.  However, it was possible for a misbehaved application to call
tj3DecompressHeader() with junk data, ignore the return value, and pass
the same junk data into tj3Transform().  Because tj3DecompressHeader()
left the global decompression state set to DSTATE_INHEADER,
tj3Transform() failed to detect the junk data (because it didn't try to
read the JPEG header), and it called jtransform_request_workspace() with
dinfo->image_width and dinfo->image_height still initialized to 0.
Because jtransform_request_workspace() does not sanity check the
decompression state, a division-by-zero error occurred with certain
combinations of transform options in which TJXOPT_TRIM or TJXOPT_CROP
was specified.  However, it should be noted that TJXOPT_TRIM and
TJXOPT_CROP cannot be expected to work properly without foreknowledge of
the JPEG source image dimensions, which cannot be gained except by
calling tj3DecompressHeader() successfully.  Thus, a calling application
is inviting trouble if it does not check the return value of
tj3DecompressHeader() and sanity check the JPEG source image dimensions
before calling tj3Transform().  This commit softens the failure, but the
failure is still due to improper API usage.
This commit is contained in:
DRC
2023-11-14 14:47:40 -05:00
parent 837e471a90
commit a27bad6552
2 changed files with 5 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
turbojpeg-mp.c
View File

@@ -174,7 +174,7 @@ DLLEXPORT int GET_NAME(tj3Decompress, BITS_IN_JSAMPLE)
retval = -1; goto bailout;
}
if (dinfo->global_state <= DSTATE_START) {
if (dinfo->global_state <= DSTATE_INHEADER) {
jpeg_mem_src_tj(dinfo, jpegBuf, jpegSize);
jpeg_read_header(dinfo, TRUE);
}

8
turbojpeg.c
View File

@@ -2345,7 +2345,7 @@ DLLEXPORT int tj3DecompressToYUVPlanes8(tjhandle handle,
retval = -1; goto bailout;
}
if (dinfo->global_state <= DSTATE_START) {
if (dinfo->global_state <= DSTATE_INHEADER) {
jpeg_mem_src_tj(dinfo, jpegBuf, jpegSize);
jpeg_read_header(dinfo, TRUE);
}
@@ -2536,7 +2536,7 @@ DLLEXPORT int tj3DecompressToYUV8(tjhandle handle,
retval = -1; goto bailout;
}
if (dinfo->global_state <= DSTATE_START) {
if (dinfo->global_state <= DSTATE_INHEADER) {
jpeg_mem_src_tj(dinfo, jpegBuf, jpegSize);
jpeg_read_header(dinfo, TRUE);
}
@@ -2683,7 +2683,7 @@ DLLEXPORT int tj3Transform(tjhandle handle, const unsigned char *jpegBuf,
retval = -1; goto bailout;
}
if (dinfo->global_state <= DSTATE_START)
if (dinfo->global_state <= DSTATE_INHEADER)
jpeg_mem_src_tj(dinfo, jpegBuf, jpegSize);
for (i = 0; i < n; i++) {
@@ -2711,7 +2711,7 @@ DLLEXPORT int tj3Transform(tjhandle handle, const unsigned char *jpegBuf,
}
jcopy_markers_setup(dinfo, saveMarkers ? JCOPYOPT_ALL : JCOPYOPT_NONE);
if (dinfo->global_state <= DSTATE_START)
if (dinfo->global_state <= DSTATE_INHEADER)
jpeg_read_header(dinfo, TRUE);
this->subsamp = getSubsamp(&this->dinfo);