diff --git a/ChangeLog.md b/ChangeLog.md index 0a075c3c..1a3e3b60 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -17,6 +17,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. +3. Fixed a long-standing issue whereby the `tjTransform()` 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, or 4:1:1 JPEG source image containing a large amount of metadata, the +issue caused `tjTransform()` 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, `tjTransform()` +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.5.1 ======= diff --git a/fuzz/transform.cc b/fuzz/transform.cc index b8a7516d..2eefd670 100644 --- a/fuzz/transform.cc +++ b/fuzz/transform.cc @@ -1,5 +1,5 @@ /* - * Copyright (C)2021 D. R. Commander. All Rights Reserved. + * Copyright (C)2021, 2023 D. R. Commander. All Rights Reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -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 }; - unsigned long dstSizes[NUMXFORMS] = { 0, 0, 0 }, maxBufSize; - int width = 0, height = 0, jpegSubsamp, jpegColorspace, i, t; - tjtransform transforms[NUMXFORMS]; + unsigned char *dstBufs[1] = { NULL }; + unsigned long dstSizes[1] = { 0 }, maxBufSize; + int width = 0, height = 0, jpegSubsamp, jpegColorspace, i; + tjtransform transforms[1]; #if defined(__has_feature) && __has_feature(memory_sanitizer) char env[18] = "JSIMD_FORCENONE=1"; @@ -67,8 +64,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; @@ -76,42 +72,71 @@ 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(tjBufSize((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; - dstBufs[2] = (unsigned char *)malloc(tjBufSize(height, width, jpegSubsamp)); - if (!dstBufs[2]) - goto bailout; - maxBufSize = tjBufSize(width, height, jpegSubsamp); - if (tjTransform(handle, data, size, NUMXFORMS, dstBufs, dstSizes, transforms, + if (tjTransform(handle, data, size, 1, dstBufs, dstSizes, transforms, TJFLAG_LIMITSCANS | TJFLAG_NOREALLOC) == 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(tjBufSize((height + 1) / 2, (width + 1) / 2, + TJSAMP_GRAY)); + if (!dstBufs[0]) + goto bailout; + + maxBufSize = tjBufSize((height + 1) / 2, (width + 1) / 2, TJSAMP_GRAY); + + if (tjTransform(handle, data, size, 1, dstBufs, dstSizes, transforms, + TJFLAG_LIMITSCANS | TJFLAG_NOREALLOC) == 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; + dstBufs[0] = (unsigned char *)malloc(tjBufSize(height, width, jpegSubsamp)); + if (!dstBufs[0]) + goto bailout; + + maxBufSize = tjBufSize(height, width, jpegSubsamp); + + if (tjTransform(handle, data, size, 1, dstBufs, dstSizes, transforms, + TJFLAG_LIMITSCANS | TJFLAG_NOREALLOC) == 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; free(dstBufs[0]); dstBufs[0] = NULL; dstSizes[0] = 0; @@ -128,8 +153,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]); if (handle) tjDestroy(handle); return 0; } diff --git a/tjbench.c b/tjbench.c index 2646eba3..7d901b8a 100644 --- a/tjbench.c +++ b/tjbench.c @@ -600,10 +600,16 @@ static int decompTest(char *fileName) if ((flags & TJFLAG_NOREALLOC) != 0 && (doTile || xformOp != TJXOP_NONE || xformOpt != 0 || customFilter)) for (i = 0; i < ntilesw * ntilesh; i++) { - if (tjBufSize(tilew, tileh, subsamp) > (unsigned long)INT_MAX) + unsigned long jpegBufSize; + + if (xformOp == TJXOP_TRANSPOSE || xformOp == TJXOP_TRANSVERSE || + xformOp == TJXOP_ROT90 || xformOp == TJXOP_ROT270) + jpegBufSize = tjBufSize(tileh, tilew, subsamp); + else + jpegBufSize = tjBufSize(tilew, tileh, subsamp); + if (jpegBufSize > (unsigned long)INT_MAX) THROW("getting buffer size", "Image is too large"); - if ((jpegBuf[i] = (unsigned char *) - tjAlloc(tjBufSize(tilew, tileh, subsamp))) == NULL) + if ((jpegBuf[i] = (unsigned char *)tjAlloc(jpegBufSize)) == NULL) THROW_UNIX("allocating JPEG tiles"); } diff --git a/turbojpeg-jni.c b/turbojpeg-jni.c index 446cbd2a..786af5c8 100644 --- a/turbojpeg-jni.c +++ b/turbojpeg-jni.c @@ -1203,6 +1203,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 b5498dc8..719a5133 100644 --- a/turbojpeg.c +++ b/turbojpeg.c @@ -2047,6 +2047,10 @@ DLLEXPORT int tjTransform(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; }