From cbfa696fc1be251a9f20f8e83e335a863a6c4be8 Mon Sep 17 00:00:00 2001 From: DRC Date: Mon, 1 Feb 2016 11:28:55 -0600 Subject: [PATCH 1/7] Update Android build instr. for ARMv8, PIE, etc. * Include information on how to do a 64-bit ARMv8 build with the latest NDK * Suggest -fPIE and -pie as default CFLAGS (required for android-16 and later. * Remove -fstrict-aliasing flag (-Wall already includes it) --- BUILDING.txt | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/BUILDING.txt b/BUILDING.txt index 06264122..c68225b4 100644 --- a/BUILDING.txt +++ b/BUILDING.txt @@ -436,18 +436,25 @@ a general recipe script that can be modified for your specific needs. # Set these variables to suit your needs NDK_PATH={full path to the "ndk" directory-- for example, /opt/android/ndk} BUILD_PLATFORM={the platform name for the NDK package you installed-- - for example, "windows-x86" or "linux-x86_64"} - TOOLCHAIN_VERSION={"4.6", "4.8", etc. This corresponds to a toolchain - directory under ${NDK_PATH}/toolchains/.} + for example, "windows-x86" or "linux-x86_64" or "darwin-x86_64"} + TOOLCHAIN_VERSION={"4.8", "4.9", "clang3.5", etc. This corresponds to a + toolchain directory under ${NDK_PATH}/toolchains/.} ANDROID_VERSION={The minimum version of Android to support-- for example, - "9", "19", etc.} + "16", "19", etc. "21" or later is required for a 64-bit build.} + # 32-bit ARMv7 build HOST=arm-linux-androideabi - TOOLCHAIN=${NDK_PATH}/toolchains/${HOST}-${TOOLCHAIN_VERSION}/prebuilt/${BUILD_PLATFORM} SYSROOT=${NDK_PATH}/platforms/android-${ANDROID_VERSION}/arch-arm - ANDROID_INCLUDES="-I${SYSROOT}/usr/include -I${TOOLCHAIN}/include" ANDROID_CFLAGS="-march=armv7-a -mfloat-abi=softfp -fprefetch-loop-arrays \ - -fstrict-aliasing --sysroot=${SYSROOT}" + --sysroot=${SYSROOT}" + + # 64-bit ARMv8 build + HOST=aarch64-linux-android + SYSROOT=${NDK_PATH}/platforms/android-${ANDROID_VERSION}/arch-arm64 + ANDROID_CFLAGS="--sysroot=${SYSROOT}" + + TOOLCHAIN=${NDK_PATH}/toolchains/${HOST}-${TOOLCHAIN_VERSION}/prebuilt/${BUILD_PLATFORM} + ANDROID_INCLUDES="-I${SYSROOT}/usr/include -I${TOOLCHAIN}/include" export CPP=${TOOLCHAIN}/bin/${HOST}-cpp export AR=${TOOLCHAIN}/bin/${HOST}-ar export AS=${TOOLCHAIN}/bin/${HOST}-as @@ -459,11 +466,14 @@ a general recipe script that can be modified for your specific needs. export STRIP=${TOOLCHAIN}/bin/${HOST}-strip cd {build_directory} sh {source_directory}/configure --host=${HOST} \ - CFLAGS="${ANDROID_INCLUDES} ${ANDROID_CFLAGS} -O3" \ + CFLAGS="${ANDROID_INCLUDES} ${ANDROID_CFLAGS} -O3 -fPIE" \ CPPFLAGS="${ANDROID_INCLUDES} ${ANDROID_CFLAGS}" \ - LDFLAGS="${ANDROID_CFLAGS}" --with-simd ${1+"$@"} + LDFLAGS="${ANDROID_CFLAGS} -pie" --with-simd ${1+"$@"} make +If building for Android 4.0.x (API level < 16) or earlier, remove -fPIE from +CFLAGS and -pie from LDFLAGS. + ******************************************************************************* ** Building on Windows (Visual C++ or MinGW) From 6e053525ee45171f65ecec596336cc3b0a5e9468 Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 09:20:41 -0600 Subject: [PATCH 2/7] TurboJPEG: Avoid dangling pointers This addresses a minor concern (LJT-01-002) expressed in a security audit by Cure53. _tjInitCompress() and _tjInitDecompress() call (respectively) jpeg_mem_dest_tj() and jpeg_mem_src_tj() with a pointer to a dummy buffer, in order to set up the destination/source manager. The dummy buffer should never be used, but it's still better to make it static so that the pointer in the destination/source manager always points to a valid region of memory. --- turbojpeg.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/turbojpeg.c b/turbojpeg.c index 6b2c6232..b20272ae 100644 --- a/turbojpeg.c +++ b/turbojpeg.c @@ -1,5 +1,5 @@ /* - * Copyright (C)2009-2015 D. R. Commander. All Rights Reserved. + * Copyright (C)2009-2016 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: @@ -556,7 +556,8 @@ DLLEXPORT unsigned char *DLLCALL tjAlloc(int bytes) static tjhandle _tjInitCompress(tjinstance *this) { - unsigned char buffer[1], *buf=buffer; unsigned long size=1; + static unsigned char buffer[1]; + unsigned char *buf=buffer; unsigned long size=1; /* This is also straight out of example.c */ this->cinfo.err=jpeg_std_error(&this->jerr.pub); @@ -1213,7 +1214,7 @@ DLLEXPORT int DLLCALL tjCompressFromYUV(tjhandle handle, unsigned char *srcBuf, static tjhandle _tjInitDecompress(tjinstance *this) { - unsigned char buffer[1]; + static unsigned char buffer[1]; /* This is also straight out of example.c */ this->dinfo.err=jpeg_std_error(&this->jerr.pub); From 271b0bf0330fbbef5d6523c518d79d8bc5787519 Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 10:08:38 -0600 Subject: [PATCH 3/7] jmemmgr.c: formatting tweaks --- jmemmgr.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/jmemmgr.c b/jmemmgr.c index f7219d26..5fbebb4b 100644 --- a/jmemmgr.c +++ b/jmemmgr.c @@ -69,9 +69,9 @@ round_up_pow2 (size_t a, size_t b) * There isn't any really portable way to determine the worst-case alignment * requirement. This module assumes that the alignment requirement is * multiples of ALIGN_SIZE. - * By default, we define ALIGN_SIZE as sizeof(double). This is necessary on some - * workstations (where doubles really do need 8-byte alignment) and will work - * fine on nearly everything. If your machine has lesser alignment needs, + * By default, we define ALIGN_SIZE as sizeof(double). This is necessary on + * some workstations (where doubles really do need 8-byte alignment) and will + * work fine on nearly everything. If your machine has lesser alignment needs, * you can save a few bytes by making ALIGN_SIZE smaller. * The only place I know of where this will NOT work is certain Macintosh * 680x0 compilers that define double as a 10-byte IEEE extended float. @@ -278,7 +278,8 @@ alloc_small (j_common_ptr cinfo, int pool_id, size_t sizeofobject) sizeofobject = round_up_pow2(sizeofobject, ALIGN_SIZE); /* Check for unsatisfiable request (do now to ensure no overflow below) */ - if ((sizeof(small_pool_hdr) + sizeofobject + ALIGN_SIZE - 1) > MAX_ALLOC_CHUNK) + if ((sizeof(small_pool_hdr) + sizeofobject + ALIGN_SIZE - 1) > + MAX_ALLOC_CHUNK) out_of_memory(cinfo, 1); /* request exceeds malloc's ability */ /* See if space is available in any existing pool */ @@ -366,7 +367,8 @@ alloc_large (j_common_ptr cinfo, int pool_id, size_t sizeofobject) sizeofobject = round_up_pow2(sizeofobject, ALIGN_SIZE); /* Check for unsatisfiable request (do now to ensure no overflow below) */ - if ((sizeof(large_pool_hdr) + sizeofobject + ALIGN_SIZE - 1) > MAX_ALLOC_CHUNK) + if ((sizeof(large_pool_hdr) + sizeofobject + ALIGN_SIZE - 1) > + MAX_ALLOC_CHUNK) out_of_memory(cinfo, 3); /* request exceeds malloc's ability */ /* Always make a new pool */ @@ -378,7 +380,8 @@ alloc_large (j_common_ptr cinfo, int pool_id, size_t sizeofobject) ALIGN_SIZE - 1); if (hdr_ptr == NULL) out_of_memory(cinfo, 4); /* jpeg_get_large failed */ - mem->total_space_allocated += sizeofobject + sizeof(large_pool_hdr) + ALIGN_SIZE - 1; + mem->total_space_allocated += sizeofobject + sizeof(large_pool_hdr) + + ALIGN_SIZE - 1; /* Success, initialize the new pool header and add to list */ hdr_ptr->next = mem->large_list[pool_id]; @@ -428,7 +431,8 @@ alloc_sarray (j_common_ptr cinfo, int pool_id, /* Make sure each row is properly aligned */ if ((ALIGN_SIZE % sizeof(JSAMPLE)) != 0) out_of_memory(cinfo, 5); /* safety check */ - samplesperrow = (JDIMENSION)round_up_pow2(samplesperrow, (2 * ALIGN_SIZE) / sizeof(JSAMPLE)); + samplesperrow = (JDIMENSION)round_up_pow2(samplesperrow, (2 * ALIGN_SIZE) / + sizeof(JSAMPLE)); /* Calculate max # of rows allowed in one allocation chunk */ ltemp = (MAX_ALLOC_CHUNK-sizeof(large_pool_hdr)) / From 6c8a71efa742c97ffac80a26859820f770e40c45 Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 10:51:22 -0600 Subject: [PATCH 4/7] rdppm.c: formatting tweaks --- rdppm.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rdppm.c b/rdppm.c index 2e9b54dd..c4aed8e8 100644 --- a/rdppm.c +++ b/rdppm.c @@ -386,7 +386,7 @@ start_input_ppm (j_compress_ptr cinfo, cjpeg_source_ptr sinfo) /* Allocate space for I/O buffer: 1 or 3 bytes or words/pixel. */ if (need_iobuffer) { source->buffer_width = (size_t) w * cinfo->input_components * - ((maxval<=255) ? sizeof(U_CHAR) : (2*sizeof(U_CHAR))); + ((maxval <= 255) ? sizeof(U_CHAR) : (2 * sizeof(U_CHAR))); source->iobuffer = (U_CHAR *) (*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_IMAGE, source->buffer_width); @@ -414,11 +414,13 @@ start_input_ppm (j_compress_ptr cinfo, cjpeg_source_ptr sinfo) /* On 16-bit-int machines we have to be careful of maxval = 65535 */ source->rescale = (JSAMPLE *) (*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_IMAGE, - (size_t) (((long) maxval + 1L) * sizeof(JSAMPLE))); + (size_t) (((long) maxval + 1L) * + sizeof(JSAMPLE))); half_maxval = maxval / 2; for (val = 0; val <= (INT32) maxval; val++) { /* The multiplication here must be done in 32 bits to avoid overflow */ - source->rescale[val] = (JSAMPLE) ((val*MAXJSAMPLE + half_maxval)/maxval); + source->rescale[val] = (JSAMPLE) ((val * MAXJSAMPLE + half_maxval) / + maxval); } } } From 3ee3d8799ae346fbf3c1e6fd15dbc1593545fa80 Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 10:58:10 -0600 Subject: [PATCH 5/7] Fix Visual C++ compiler warnings --- jcdctmgr.c | 2 +- rdppm.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jcdctmgr.c b/jcdctmgr.c index 4cac6664..6a8e8311 100644 --- a/jcdctmgr.c +++ b/jcdctmgr.c @@ -184,7 +184,7 @@ compute_reciprocal (UINT16 divisor, DCTELEM * dtbl) dtbl[DCTSIZE2 * 0] = (DCTELEM) 1; /* reciprocal */ dtbl[DCTSIZE2 * 1] = (DCTELEM) 0; /* correction */ dtbl[DCTSIZE2 * 2] = (DCTELEM) 1; /* scale */ - dtbl[DCTSIZE2 * 3] = (DCTELEM) (-sizeof(DCTELEM) * 8); /* shift */ + dtbl[DCTSIZE2 * 3] = -(DCTELEM) (sizeof(DCTELEM) * 8); /* shift */ return 0; } diff --git a/rdppm.c b/rdppm.c index c4aed8e8..ebe82acc 100644 --- a/rdppm.c +++ b/rdppm.c @@ -92,7 +92,7 @@ pbm_getc (FILE * infile) LOCAL(unsigned int) -read_pbm_integer (j_compress_ptr cinfo, FILE * infile, int maxval) +read_pbm_integer (j_compress_ptr cinfo, FILE * infile, unsigned int maxval) /* Read an unsigned decimal integer from the PPM file */ /* Swallows one trailing character after the integer */ /* Note that on a 16-bit-int machine, only values up to 64k can be read. */ @@ -144,7 +144,7 @@ get_text_gray_row (j_compress_ptr cinfo, cjpeg_source_ptr sinfo) register JSAMPROW ptr; register JSAMPLE *rescale = source->rescale; JDIMENSION col; - int maxval = source->maxval; + unsigned int maxval = source->maxval; ptr = source->pub.buffer[0]; for (col = cinfo->image_width; col > 0; col--) { @@ -163,7 +163,7 @@ get_text_rgb_row (j_compress_ptr cinfo, cjpeg_source_ptr sinfo) register JSAMPROW ptr; register JSAMPLE *rescale = source->rescale; JDIMENSION col; - int maxval = source->maxval; + unsigned int maxval = source->maxval; ptr = source->pub.buffer[0]; for (col = cinfo->image_width; col > 0; col--) { From 04dd34c14ed21d36e80447dd988fa1ce4ebe5ac5 Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 10:59:21 -0600 Subject: [PATCH 6/7] Guard against wrap-around in alloc functions Because of the exposed nature of the libjpeg API, alloc_small() and alloc_large() can potentially be called by external code. If an application were to call either of those functions with sizeofobject > SIZE_MAX - ALIGN_SIZE - 1, then the math in round_up_pow2() would wrap around to zero, causing that function to return a small value. That value would likely not exceed MAX_ALLOC_CHUNK, so the subsequent size checks in alloc_small() and alloc_large() would not catch the error. A similar problem could occur in 32-bit builds if alloc_sarray() were called with samplesperrow > SIZE_MAX - (2 * ALIGN_SIZE / sizeof(JSAMPLE)) - 1 This patch simply ensures that the size argument to the alloc_*() functions will never exceed MAX_ALLOC_CHUNK (1 billion). If it did, then subsequent size checks would eventually catch that error, so we are instead catching the error before round_up_pow2() is called. This addresses a minor concern (LJT-01-001) expressed in a security audit by Cure53. --- jmemmgr.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/jmemmgr.c b/jmemmgr.c index 5fbebb4b..4b0fcac0 100644 --- a/jmemmgr.c +++ b/jmemmgr.c @@ -3,8 +3,8 @@ * * This file was part of the Independent JPEG Group's software: * Copyright (C) 1991-1997, Thomas G. Lane. - * It was modified by The libjpeg-turbo Project to include only code and - * information relevant to libjpeg-turbo. + * libjpeg-turbo Modifications: + * Copyright (C) 2016, D. R. Commander. * For conditions of distribution and use, see the accompanying README file. * * This file contains the JPEG system-independent memory management @@ -275,6 +275,11 @@ alloc_small (j_common_ptr cinfo, int pool_id, size_t sizeofobject) * and so that algorithms can straddle outside the proper area up * to the next alignment. */ + if (sizeofobject > MAX_ALLOC_CHUNK) { + /* This prevents overflow/wrap-around in round_up_pow2() if sizeofobject + is close to SIZE_MAX. */ + out_of_memory(cinfo, 7); + } sizeofobject = round_up_pow2(sizeofobject, ALIGN_SIZE); /* Check for unsatisfiable request (do now to ensure no overflow below) */ @@ -364,6 +369,11 @@ alloc_large (j_common_ptr cinfo, int pool_id, size_t sizeofobject) * algorithms can straddle outside the proper area up to the next * alignment. */ + if (sizeofobject > MAX_ALLOC_CHUNK) { + /* This prevents overflow/wrap-around in round_up_pow2() if sizeofobject + is close to SIZE_MAX. */ + out_of_memory(cinfo, 8); + } sizeofobject = round_up_pow2(sizeofobject, ALIGN_SIZE); /* Check for unsatisfiable request (do now to ensure no overflow below) */ @@ -431,6 +441,12 @@ alloc_sarray (j_common_ptr cinfo, int pool_id, /* Make sure each row is properly aligned */ if ((ALIGN_SIZE % sizeof(JSAMPLE)) != 0) out_of_memory(cinfo, 5); /* safety check */ + + if (samplesperrow > MAX_ALLOC_CHUNK) { + /* This prevents overflow/wrap-around in round_up_pow2() if sizeofobject + is close to SIZE_MAX. */ + out_of_memory(cinfo, 9); + } samplesperrow = (JDIMENSION)round_up_pow2(samplesperrow, (2 * ALIGN_SIZE) / sizeof(JSAMPLE)); From 0463f7c9aad060fcd56e98d025ce16185279e2bc Mon Sep 17 00:00:00 2001 From: DRC Date: Thu, 4 Feb 2016 18:34:38 -0600 Subject: [PATCH 7/7] Prevent overread when decoding malformed JPEG The accelerated Huffman decoder was previously invoked if there were > 128 bytes in the input buffer. However, it is possible to construct a JPEG image with Huffman blocks > 430 bytes in length (http://stackoverflow.com/questions/2734678/jpeg-calculating-max-size). While such images are pathological and could never be created by a JPEG compressor, it is conceivable that an attacker could use such an artifially-constructed image to trigger an input buffer overrun in the libjpeg-turbo decompressor and thus gain access to some of the data on the calling program's heap. This patch simply increases the minimum buffer size for the accelerated Huffman decoder to 512 bytes, which should (hopefully) accommodate any possible input. This addresses a major issue (LJT-01-005) identified in a security audit by Cure53. --- ChangeLog.txt | 9 +++++++++ jdhuff.c | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 28aca8ad..3b6d05b7 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -39,6 +39,15 @@ set to JMSG_COPYRIGHT. [10] Fixed a bug in the build system that was causing the Windows version of wrjpgcom to be built using the rdjpgcom code. +[11] Fixed an issue in the accelerated Huffman decoder that could have caused +the decoder to read past the end of the input buffer when a malformed, +specially-crafted JPEG image was being decompressed. In prior versions of +libjpeg-turbo, the accelerated Huffman decoder was invoked (in most cases) only +if there were > 128 bytes of data in the input buffer. However, it is possible +to construct a JPEG image in which a single Huffman block is over 430 bytes +long, so this version of libjpeg-turbo activates the accelerated Huffman +decoder only if there are > 512 bytes of data in the input buffer. + 1.2.1 ===== diff --git a/jdhuff.c b/jdhuff.c index 4197cc5c..59b065a1 100644 --- a/jdhuff.c +++ b/jdhuff.c @@ -4,7 +4,7 @@ * This file was part of the Independent JPEG Group's software: * Copyright (C) 1991-1997, Thomas G. Lane. * Modifications: - * Copyright (C) 2009-2011, D. R. Commander. + * Copyright (C) 2009-2011, 2016, D. R. Commander. * For conditions of distribution and use, see the accompanying README file. * * This file contains Huffman entropy decoding routines. @@ -743,7 +743,7 @@ decode_mcu_fast (j_decompress_ptr cinfo, JBLOCKROW *MCU_data) * this module, since we'll just re-assign them on the next call.) */ -#define BUFSIZE (DCTSIZE2 * 2) +#define BUFSIZE (DCTSIZE2 * 8) METHODDEF(boolean) decode_mcu (j_decompress_ptr cinfo, JBLOCKROW *MCU_data)