diff --git a/ChangeLog.md b/ChangeLog.md index b9d387a7..c61565be 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -29,6 +29,22 @@ memory. With 12-bit data precision, this caused a buffer overrun or underrun and subsequent segfault if the sample value read from unitialized memory was outside of the valid sample range. +5. Fixed a long-standing issue whereby the `tj3Transform()` function, when used +with the `TJXOP_TRANSPOSE`, `TJXOP_TRANSVERSE`, `TJXOP_ROT90`, or +`TJXOP_ROT270` transform operation and without automatic JPEG destination +buffer (re)allocation or lossless cropping, computed the worst-case transformed +JPEG image size based on the source image dimensions rather than the +transformed image dimensions. If a calling program allocated the JPEG +destination buffer based on the transformed image dimensions, as the API +documentation instructs, and attempted to transform a specially-crafted 4:2:2, +4:4:0, 4:1:1, or 4:4:1 JPEG source image containing a large amount of metadata, +the issue caused `tj3Transform()` to overflow the JPEG destination buffer +rather than fail gracefully. The issue could be worked around by setting +`TJXOPT_COPYNONE`. Note that, irrespective of this issue, `tj3Transform()` +cannot reliably transform JPEG source images that contain a large amount of +metadata unless automatic JPEG destination buffer (re)allocation is used or +`TJXOPT_COPYNONE` is set. + 2.1.91 (3.0 beta2) ================== diff --git a/fuzz/transform.cc b/fuzz/transform.cc index fe4a6a21..b3458db5 100644 --- a/fuzz/transform.cc +++ b/fuzz/transform.cc @@ -32,16 +32,13 @@ #include -#define NUMXFORMS 3 - - extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { tjhandle handle = NULL; - unsigned char *dstBufs[NUMXFORMS] = { NULL, NULL, NULL }; - size_t dstSizes[NUMXFORMS] = { 0, 0, 0 }, maxBufSize; - int width = 0, height = 0, jpegSubsamp, i, t; - tjtransform transforms[NUMXFORMS]; + unsigned char *dstBufs[1] = { NULL }; + size_t dstSizes[1] = { 0 }, maxBufSize; + int width = 0, height = 0, jpegSubsamp, i; + tjtransform transforms[1]; #if defined(__has_feature) && __has_feature(memory_sanitizer) char env[18] = "JSIMD_FORCENONE=1"; @@ -69,8 +66,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) if (jpegSubsamp < 0 || jpegSubsamp >= TJ_NUMSAMP) jpegSubsamp = TJSAMP_444; - for (t = 0; t < NUMXFORMS; t++) - memset(&transforms[t], 0, sizeof(tjtransform)); + memset(&transforms[0], 0, sizeof(tjtransform)); transforms[0].op = TJXOP_NONE; transforms[0].options = TJXOPT_PROGRESSIVE | TJXOPT_COPYNONE; @@ -79,44 +75,73 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) if (!dstBufs[0]) goto bailout; - transforms[1].r.w = (width + 1) / 2; - transforms[1].r.h = (height + 1) / 2; - transforms[1].op = TJXOP_TRANSPOSE; - transforms[1].options = TJXOPT_GRAY | TJXOPT_CROP | TJXOPT_COPYNONE; - dstBufs[1] = - (unsigned char *)malloc(tj3JPEGBufSize((width + 1) / 2, (height + 1) / 2, - TJSAMP_GRAY)); - if (!dstBufs[1]) - goto bailout; - - transforms[2].op = TJXOP_ROT90; - transforms[2].options = TJXOPT_TRIM | TJXOPT_COPYNONE | TJXOPT_ARITHMETIC; - dstBufs[2] = - (unsigned char *)malloc(tj3JPEGBufSize(height, width, jpegSubsamp)); - if (!dstBufs[2]) - goto bailout; - maxBufSize = tj3JPEGBufSize(width, height, jpegSubsamp); tj3Set(handle, TJPARAM_NOREALLOC, 1); - if (tj3Transform(handle, data, size, NUMXFORMS, dstBufs, dstSizes, + if (tj3Transform(handle, data, size, 1, dstBufs, dstSizes, transforms) == 0) { /* Touch all of the output pixels in order to catch uninitialized reads when using MemorySanitizer. */ - for (t = 0; t < NUMXFORMS; t++) { - int sum = 0; + int sum = 0; - for (i = 0; i < dstSizes[t]; i++) - sum += dstBufs[t][i]; + for (i = 0; i < dstSizes[0]; i++) + sum += dstBufs[0][i]; - /* Prevent the code above from being optimized out. This test should - never be true, but the compiler doesn't know that. */ - if (sum > 255 * maxBufSize) - goto bailout; - } + /* Prevent the code above from being optimized out. This test should + never be true, but the compiler doesn't know that. */ + if (sum > 255 * maxBufSize) + goto bailout; + } + + free(dstBufs[0]); + dstBufs[0] = NULL; + + transforms[0].r.w = (height + 1) / 2; + transforms[0].r.h = (width + 1) / 2; + transforms[0].op = TJXOP_TRANSPOSE; + transforms[0].options = TJXOPT_GRAY | TJXOPT_CROP | TJXOPT_COPYNONE; + dstBufs[0] = + (unsigned char *)malloc(tj3JPEGBufSize((height + 1) / 2, (width + 1) / 2, + TJSAMP_GRAY)); + if (!dstBufs[0]) + goto bailout; + + maxBufSize = tj3JPEGBufSize((height + 1) / 2, (width + 1) / 2, TJSAMP_GRAY); + + if (tj3Transform(handle, data, size, 1, dstBufs, dstSizes, + transforms) == 0) { + int sum = 0; + + for (i = 0; i < dstSizes[0]; i++) + sum += dstBufs[0][i]; + + if (sum > 255 * maxBufSize) + goto bailout; + } + + free(dstBufs[0]); + dstBufs[0] = NULL; + + transforms[0].op = TJXOP_ROT90; + transforms[0].options = TJXOPT_TRIM | TJXOPT_ARITHMETIC; + dstBufs[0] = + (unsigned char *)malloc(tj3JPEGBufSize(height, width, jpegSubsamp)); + if (!dstBufs[0]) + goto bailout; + + maxBufSize = tj3JPEGBufSize(height, width, jpegSubsamp); + + if (tj3Transform(handle, data, size, 1, dstBufs, dstSizes, + transforms) == 0) { + int sum = 0; + + for (i = 0; i < dstSizes[0]; i++) + sum += dstBufs[0][i]; + + if (sum > 255 * maxBufSize) + goto bailout; } - transforms[0].options &= ~TJXOPT_COPYNONE; transforms[0].options |= TJXOPT_OPTIMIZE; free(dstBufs[0]); dstBufs[0] = NULL; @@ -135,8 +160,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) } bailout: - for (t = 0; t < NUMXFORMS; t++) - free(dstBufs[t]); + free(dstBufs[0]); tj3Destroy(handle); return 0; } diff --git a/tjbench.c b/tjbench.c index db08f060..e053dc6d 100644 --- a/tjbench.c +++ b/tjbench.c @@ -722,8 +722,13 @@ static int decompTest(char *fileName) if (noRealloc && (doTile || xformOp != TJXOP_NONE || xformOpt != 0 || customFilter)) { for (i = 0; i < ntilesw * ntilesh; i++) { - size_t jpegBufSize = tj3JPEGBufSize(tilew, tileh, subsamp); + size_t jpegBufSize; + if (xformOp == TJXOP_TRANSPOSE || xformOp == TJXOP_TRANSVERSE || + xformOp == TJXOP_ROT90 || xformOp == TJXOP_ROT270) + jpegBufSize = tj3JPEGBufSize(tileh, tilew, subsamp); + else + jpegBufSize = tj3JPEGBufSize(tilew, tileh, subsamp); if (jpegBufSize == 0) THROW_TJG(); if ((jpegBufs[i] = tj3Alloc(jpegBufSize)) == NULL) diff --git a/turbojpeg-jni.c b/turbojpeg-jni.c index d9531a25..32186f3f 100644 --- a/turbojpeg-jni.c +++ b/turbojpeg-jni.c @@ -1213,6 +1213,10 @@ JNIEXPORT jintArray JNICALL Java_org_libjpegturbo_turbojpeg_TJTransformer_transf for (i = 0; i < n; i++) { int w = jpegWidth, h = jpegHeight; + if (t[i].op == TJXOP_TRANSPOSE || t[i].op == TJXOP_TRANSVERSE || + t[i].op == TJXOP_ROT90 || t[i].op == TJXOP_ROT270) { + w = jpegHeight; h = jpegWidth; + } if (t[i].r.w != 0) w = t[i].r.w; if (t[i].r.h != 0) h = t[i].r.h; BAILIF0(jdstBufs[i] = (*env)->GetObjectArrayElement(env, dstobjs, i)); diff --git a/turbojpeg.c b/turbojpeg.c index 32344466..b75d4148 100644 --- a/turbojpeg.c +++ b/turbojpeg.c @@ -2702,6 +2702,10 @@ DLLEXPORT int tj3Transform(tjhandle handle, const unsigned char *jpegBuf, if (!xinfo[i].crop) { w = dinfo->image_width; h = dinfo->image_height; + if (t[i].op == TJXOP_TRANSPOSE || t[i].op == TJXOP_TRANSVERSE || + t[i].op == TJXOP_ROT90 || t[i].op == TJXOP_ROT270) { + w = dinfo->image_height; h = dinfo->image_width; + } } else { w = xinfo[i].crop_width; h = xinfo[i].crop_height; }