From 736babd599f9c3dd296c2f168b8eb12f50402770 Mon Sep 17 00:00:00 2001 From: Joaquin Anton Date: Fri, 5 Aug 2022 18:42:25 +0200 Subject: [PATCH] Fix memory leak in libjpeg-turbo decoder implementation in case of corrupted images (#4138) Signed-off-by: Joaquin Anton --- dali/image/jpeg.cc | 24 ++-- dali/image/jpeg_mem.cc | 297 +++++------------------------------------ dali/image/jpeg_mem.h | 40 +----- 3 files changed, 47 insertions(+), 314 deletions(-) diff --git a/dali/image/jpeg.cc b/dali/image/jpeg.cc index 9a25afc4a8..1bd883c0de 100644 --- a/dali/image/jpeg.cc +++ b/dali/image/jpeg.cc @@ -68,6 +68,7 @@ JpegImage::DecodeImpl(DALIImageType type, const uint8 *jpeg, size_t length) cons type = shape[2] == 3 ? DALI_RGB : DALI_GRAY; } const auto c = NumberOfChannels(type); + Image::Shape target_shape{h, w, c}; DALI_ENFORCE(jpeg != nullptr); DALI_ENFORCE(length > 0); @@ -97,32 +98,23 @@ JpegImage::DecodeImpl(DALIImageType type, const uint8 *jpeg, size_t length) cons flags.crop_x = crop.anchor[1]; flags.crop_height = crop.shape[0]; flags.crop_width = crop.shape[1]; + target_shape[0] = crop.shape[0]; + target_shape[1] = crop.shape[1]; } DALI_ENFORCE(type == DALI_RGB || type == DALI_BGR || type == DALI_GRAY, "Color space not supported by libjpeg-turbo"); flags.color_space = type; - std::shared_ptr decoded_image; - int cropped_h = 0; - int cropped_w = 0; - uint8_t* result = jpeg::Uncompress( - jpeg, length, flags, nullptr /* nwarn */, - [&decoded_image, &cropped_h, &cropped_w](int width, int height, int channels) -> uint8* { - decoded_image.reset( - new uint8_t[height * width * channels], - [](uint8_t* data){ delete [] data; } ); - cropped_h = height; - cropped_w = width; - return decoded_image.get(); - }); - - if (result == nullptr) { + std::shared_ptr decoded_image(jpeg::Uncompress(jpeg, length, flags).release(), + [](uint8_t *data) { delete[] data; }); + + if (decoded_image == nullptr) { // Failed to decode, fallback return GenericImage::DecodeImpl(type, jpeg, length); } - return {decoded_image, {cropped_h, cropped_w, c}}; + return {decoded_image, target_shape}; #else // DALI_USE_JPEG_TURBO return GenericImage::DecodeImpl(type, jpeg, length); #endif // DALI_USE_JPEG_TURBO diff --git a/dali/image/jpeg_mem.cc b/dali/image/jpeg_mem.cc index d018e564a3..afaa441a93 100644 --- a/dali/image/jpeg_mem.cc +++ b/dali/image/jpeg_mem.cc @@ -27,6 +27,7 @@ limitations under the License. #include #include "dali/image/jpeg_handle.h" #include "dali/core/error_handling.h" +#include "dali/core/call_at_exit.h" namespace dali { namespace jpeg { @@ -46,22 +47,16 @@ enum JPEGErrors { // arguments in a struct struct. class FewerArgsForCompiler { public: - FewerArgsForCompiler(int datasize, const UncompressFlags& flags, int64* nwarn, - std::function allocate_output) + FewerArgsForCompiler(int datasize, const UncompressFlags& flags) : datasize_(datasize), flags_(flags), - pnwarn_(nwarn), - allocate_output_(std::move(allocate_output)), height_read_(0), height_(0), stride_(0) { - if (pnwarn_ != nullptr) *pnwarn_ = 0; } const int datasize_; const UncompressFlags flags_; - int64* const pnwarn_; - std::function allocate_output_; int height_read_; // number of scanline lines successfully read int height_; int stride_; @@ -78,14 +73,13 @@ bool IsCropWindowValid(const UncompressFlags& flags, int input_image_width, flags.crop_x + flags.crop_width <= input_image_width; } -uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { +std::unique_ptr UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { // unpack the argball const int datasize = argball->datasize_; const auto& flags = argball->flags_; const int ratio = flags.ratio; int components = flags.components; int stride = flags.stride; // may be 0 - int64* const nwarn = argball->pnwarn_; // may be NULL auto color_space = flags.color_space; // Can't decode if the ratio is not recognized by libjpeg @@ -102,7 +96,8 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { if (datasize == 0 || srcdata == nullptr) return nullptr; // Declare temporary buffer pointer here so that we can free on error paths - JSAMPLE* tempdata = nullptr; + std::unique_ptr temp; + JSAMPLE *tempdata = nullptr; // Initialize libjpeg structures to have a memory source // Modify the usual jpeg error manager to catch fatal errors. @@ -114,11 +109,14 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { cinfo.client_data = &jpeg_jmpbuf; jerr.error_exit = CatchError; if (setjmp(jpeg_jmpbuf)) { - delete[] tempdata; return nullptr; } jpeg_create_decompress(&cinfo); + auto destroy_cinfo = AtScopeExit([&cinfo]() { + jpeg_destroy_decompress(&cinfo); + }); + SetSrc(&cinfo, srcdata, datasize, flags.try_recover_truncated_jpeg); jpeg_read_header(&cinfo, TRUE); @@ -136,7 +134,6 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { break; default: ERROR_LOG << " Invalid components value " << components << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } @@ -164,12 +161,10 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { if (cinfo.output_width <= 0 || cinfo.output_height <= 0) { ERROR_LOG << "Invalid image size: " << cinfo.output_width << " x " << cinfo.output_height << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } if (total_size >= (1LL << 29)) { ERROR_LOG << "Image too large: " << total_size << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } @@ -194,7 +189,6 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { << " for image_width: " << cinfo.output_width << " and image_height: " << cinfo.output_height << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } @@ -227,7 +221,6 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { } else if (stride < min_stride) { ERROR_LOG << "Incompatible stride: " << stride << " < " << min_stride << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } @@ -235,23 +228,22 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { argball->height_ = target_output_height; argball->stride_ = stride; + + std::unique_ptr dstdata; #if !defined(LIBJPEG_TURBO_VERSION) - uint8* dstdata = nullptr; - if (flags.crop) { - dstdata = new JSAMPLE[stride * target_output_height]; - } else { - dstdata = argball->allocate_output_(target_output_width, - target_output_height, components); - } + if (flags.crop) + dstdata.reset(new JSAMPLE[stride * target_output_height]); + else + dstdata.reset(new JSAMPLE[target_output_width * target_output_height * components]); #else - uint8* dstdata = argball->allocate_output_(target_output_width, - target_output_height, components); + dstdata.reset(new JSAMPLE[target_output_width * target_output_height * components]); #endif + if (dstdata == nullptr) { - jpeg_destroy_decompress(&cinfo); return nullptr; } - JSAMPLE* output_line = static_cast(dstdata); + + JSAMPLE* output_line = static_cast(dstdata.get()); // jpeg_read_scanlines requires the buffers to be allocated based on // cinfo.output_width, but the target image width might be different if crop @@ -265,11 +257,12 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { if (use_cmyk) { // Temporary buffer used for CMYK -> RGB conversion. - tempdata = new JSAMPLE[cinfo.output_width * 4]; + temp.reset(new JSAMPLE[cinfo.output_width * 4]); } else if (need_realign_cropped_scanline) { // Temporary buffer used for MCU-aligned scanline data. - tempdata = new JSAMPLE[cinfo.output_width * components]; + temp.reset(new JSAMPLE[cinfo.output_width * components]); } + tempdata = temp.get(); // If there is an error reading a line, this aborts the reading. // Save the fraction of the image that has been read. @@ -363,7 +356,7 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { DALI_ENFORCE(num_lines_read == 1); output_line += stride; } - delete[] tempdata; + temp.reset(); tempdata = nullptr; #if defined(LIBJPEG_TURBO_VERSION) @@ -383,7 +376,7 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { if (components == 4) { // Start on the last line. JSAMPLE* scanlineptr = static_cast( - dstdata + static_cast(target_output_height - 1) * stride); + dstdata.get() + static_cast(target_output_height - 1) * stride); const JSAMPLE kOpaque = -1; // All ones appropriate for JSAMPLE. const int right_rgb = (target_output_width - 1) * 3; const int right_rgba = (target_output_width - 1) * 4; @@ -426,15 +419,9 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { default: // will never happen, should be catched by the previous switch ERROR_LOG << "Invalid components value " << components << std::endl; - jpeg_destroy_decompress(&cinfo); return nullptr; } - // save number of warnings if requested - if (nwarn != nullptr) { - *nwarn = cinfo.err->num_warnings; - } - // Handle errors in JPEG switch (error) { case JPEGERRORS_OK: @@ -465,17 +452,13 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { << " for image_width: " << cinfo.output_width << " and image_height: " << cinfo.output_height << std::endl; - delete[] dstdata; - jpeg_destroy_decompress(&cinfo); return nullptr; } - const uint8* full_image = dstdata; - dstdata = argball->allocate_output_(target_output_width, - target_output_height, components); + const auto full_image = std::move(dstdata); + dstdata = std::unique_ptr( + new JSAMPLE[target_output_width, target_output_height, components]); if (dstdata == nullptr) { - delete[] full_image; - jpeg_destroy_decompress(&cinfo); return nullptr; } @@ -492,18 +475,16 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { argball->height_read_ = target_output_height; } const int crop_offset = flags.crop_x * components * sizeof(JSAMPLE); - const uint8* full_image_ptr = full_image + flags.crop_y * full_image_stride; - uint8* crop_image_ptr = dstdata; + const uint8* full_image_ptr = full_image.get() + flags.crop_y * full_image_stride; + uint8* crop_image_ptr = dstdata.get(); for (int i = 0; i < argball->height_read_; i++) { memcpy(crop_image_ptr, full_image_ptr + crop_offset, min_stride); crop_image_ptr += stride; full_image_ptr += full_image_stride; } - delete[] full_image; } #endif - jpeg_destroy_decompress(&cinfo); return dstdata; } @@ -517,12 +498,10 @@ uint8* UncompressLow(const void* srcdata, FewerArgsForCompiler* argball) { // associated libraries aren't good enough to guarantee that 7 // parameters won't get clobbered by the longjmp. So we help // it out a little. -uint8* Uncompress(const void* srcdata, int datasize, - const UncompressFlags& flags, int64* nwarn, - std::function allocate_output) { - FewerArgsForCompiler argball(datasize, flags, nwarn, - std::move(allocate_output)); - uint8* const dstdata = UncompressLow(srcdata, &argball); +std::unique_ptr Uncompress(const void* srcdata, int datasize, + const UncompressFlags& flags) { + FewerArgsForCompiler argball(datasize, flags); + auto dstdata = UncompressLow(srcdata, &argball); const float fraction_read = argball.height_ == 0 @@ -538,7 +517,7 @@ uint8* Uncompress(const void* srcdata, int datasize, // set the unread pixels to black if (argball.height_read_ != argball.height_) { const int first_bad_line = argball.height_read_; - uint8* start = dstdata + first_bad_line * argball.stride_; + uint8* start = dstdata.get() + first_bad_line * argball.stride_; const int nbytes = (argball.height_ - first_bad_line) * argball.stride_; memset(static_cast(start), 0, nbytes); } @@ -546,23 +525,6 @@ uint8* Uncompress(const void* srcdata, int datasize, return dstdata; } -uint8* Uncompress(const void* srcdata, int datasize, - const UncompressFlags& flags, int* pwidth, int* pheight, - int* pcomponents, int64* nwarn) { - uint8* buffer = nullptr; - uint8* result = - Uncompress(srcdata, datasize, flags, nwarn, - [=, &buffer](int width, int height, int components) { - if (pwidth != nullptr) *pwidth = width; - if (pheight != nullptr) *pheight = height; - if (pcomponents != nullptr) *pcomponents = components; - buffer = new uint8[height * width * components]; - return buffer; - }); - if (!result) delete[] buffer; - return result; -} - // ---------------------------------------------------------------------------- // Computes image information from jpeg header. // Returns true on success; false on failure. @@ -606,196 +568,5 @@ bool GetImageInfo(const void* srcdata, int datasize, int* width, int* height, return true; } -// ----------------------------------------------------------------------------- -// Compression - -namespace { -bool CompressInternal(const uint8* srcdata, int width, int height, - const CompressFlags& flags, string* output) { - output->clear(); - const int components = (static_cast(flags.format) & 0xff); - - int64 total_size = static_cast(width) * static_cast(height); - // Some of the internal routines do not gracefully handle ridiculously - // large images, so fail fast. - if (width <= 0 || height <= 0) { - ERROR_LOG << "Invalid image size: " << width << " x " << height << std::endl; - return false; - } - if (total_size >= (1_i64 << 29)) { - ERROR_LOG << "Image too large: " << total_size << std::endl; - return false; - } - - int in_stride = flags.stride; - if (in_stride == 0) { - in_stride = width * (static_cast(flags.format) & 0xff); - } else if (in_stride < width * components) { - ERROR_LOG << "Incompatible input stride" << std::endl; - return false; - } - - JOCTET* buffer = nullptr; - - // NOTE: for broader use xmp_metadata should be made a unicode string - DALI_ENFORCE(srcdata != nullptr); - DALI_ENFORCE(output != nullptr); - // This struct contains the JPEG compression parameters and pointers to - // working space - struct jpeg_compress_struct cinfo; - // This struct represents a JPEG error handler. - struct jpeg_error_mgr jerr; - jmp_buf jpeg_jmpbuf; // recovery point in case of error - - // Step 1: allocate and initialize JPEG compression object - // Use the usual jpeg error manager. - cinfo.err = jpeg_std_error(&jerr); - cinfo.client_data = &jpeg_jmpbuf; - jerr.error_exit = CatchError; - if (setjmp(jpeg_jmpbuf)) { - output->clear(); - delete[] buffer; - return false; - } - - jpeg_create_compress(&cinfo); - - // Step 2: specify data destination - // We allocate a buffer of reasonable size. If we have a small image, just - // estimate the size of the output using the number of bytes of the input. - // If this is getting too big, we will append to the string by chunks of 1MB. - // This seems like a reasonable compromise between performance and memory. - int bufsize = std::min(width * height * components, 1 << 20); - buffer = new JOCTET[bufsize]; - SetDest(&cinfo, buffer, bufsize, output); - - // Step 3: set parameters for compression - cinfo.image_width = width; - cinfo.image_height = height; - switch (components) { - case 1: - cinfo.input_components = 1; - cinfo.in_color_space = JCS_GRAYSCALE; - break; - case 3: - case 4: - cinfo.input_components = 3; - cinfo.in_color_space = JCS_RGB; - break; - default: - ERROR_LOG << " Invalid components value " << components << std::endl; - output->clear(); - delete[] buffer; - return false; - } - jpeg_set_defaults(&cinfo); - if (flags.optimize_jpeg_size) cinfo.optimize_coding = TRUE; - - cinfo.density_unit = flags.density_unit; // JFIF code for pixel size units: - // 1 = in, 2 = cm - cinfo.X_density = flags.x_density; // Horizontal pixel density - cinfo.Y_density = flags.y_density; // Vertical pixel density - jpeg_set_quality(&cinfo, flags.quality, TRUE); - - if (flags.progressive) { - jpeg_simple_progression(&cinfo); - } - - if (!flags.chroma_downsampling) { - // Turn off chroma subsampling (it is on by default). For more details on - // chroma subsampling, see http://en.wikipedia.org/wiki/Chroma_subsampling. - for (int i = 0; i < cinfo.num_components; ++i) { - cinfo.comp_info[i].h_samp_factor = 1; - cinfo.comp_info[i].v_samp_factor = 1; - } - } - - jpeg_start_compress(&cinfo, TRUE); - - // Embed XMP metadata if any - if (!flags.xmp_metadata.empty()) { - // XMP metadata is embedded in the APP1 tag of JPEG and requires this - // namespace header string (null-terminated) - const string name_space = "http://ns.adobe.com/xap/1.0/"; - const int name_space_length = name_space.size(); - const int metadata_length = flags.xmp_metadata.size(); - const int packet_length = metadata_length + name_space_length + 1; - std::unique_ptr joctet_packet(new JOCTET[packet_length]); - - for (int i = 0; i < name_space_length; i++) { - // Conversion char --> JOCTET - joctet_packet[i] = name_space[i]; - } - joctet_packet[name_space_length] = 0; // null-terminate namespace string - - for (int i = 0; i < metadata_length; i++) { - // Conversion char --> JOCTET - joctet_packet[i + name_space_length + 1] = flags.xmp_metadata[i]; - } - jpeg_write_marker(&cinfo, JPEG_APP0 + 1, joctet_packet.get(), - packet_length); - } - - // JSAMPLEs per row in image_buffer - std::unique_ptr row_temp( - new JSAMPLE[width * cinfo.input_components]); - while (cinfo.next_scanline < cinfo.image_height) { - JSAMPROW row_pointer[1]; // pointer to JSAMPLE row[s] - const uint8* r = &srcdata[cinfo.next_scanline * in_stride]; - uint8* p = static_cast(row_temp.get()); - switch (flags.format) { - case FORMAT_RGBA: { - for (int i = 0; i < width; ++i, p += 3, r += 4) { - p[0] = r[0]; - p[1] = r[1]; - p[2] = r[2]; - } - row_pointer[0] = row_temp.get(); - break; - } - case FORMAT_ABGR: { - for (int i = 0; i < width; ++i, p += 3, r += 4) { - p[0] = r[3]; - p[1] = r[2]; - p[2] = r[1]; - } - row_pointer[0] = row_temp.get(); - break; - } - default: { - row_pointer[0] = reinterpret_cast(const_cast(r)); - } - } - - DALI_ENFORCE( - jpeg_write_scanlines(&cinfo, row_pointer, 1) == 1u); - } - jpeg_finish_compress(&cinfo); - - // release JPEG compression object - jpeg_destroy_compress(&cinfo); - delete[] buffer; - return true; -} - -} // anonymous namespace - -// ----------------------------------------------------------------------------- - -bool Compress(const void* srcdata, int width, int height, - const CompressFlags& flags, string* output) { - return CompressInternal(static_cast(srcdata), width, height, - flags, output); -} - -string Compress(const void* srcdata, int width, int height, - const CompressFlags& flags) { - string temp; - CompressInternal(static_cast(srcdata), width, height, flags, - &temp); - // If CompressInternal fails, temp will be empty. - return temp; -} - } // namespace jpeg } // namespace dali diff --git a/dali/image/jpeg_mem.h b/dali/image/jpeg_mem.h index 8628f9480a..2e2d12284f 100644 --- a/dali/image/jpeg_mem.h +++ b/dali/image/jpeg_mem.h @@ -24,6 +24,7 @@ limitations under the License. #define DALI_IMAGE_JPEG_MEM_H_ #include +#include #include #include "dali/core/common.h" #include "dali/image/jpeg_utils.h" @@ -79,29 +80,10 @@ struct UncompressFlags { // Uncompress some raw JPEG data given by the pointer srcdata and the length // datasize. -// - width and height are the address where to store the size of the -// uncompressed image in pixels. May be nullptr. -// - components is the address where the number of read components are -// stored. This is *output only*: to request a specific number of -// components use flags.components. May be nullptr. -// - nwarn is the address in which to store the number of warnings. -// May be nullptr. -// The function returns a pointer to the raw uncompressed data or NULL if -// there was an error. The caller of the function is responsible for -// freeing the memory (using delete []). -uint8* Uncompress(const void* srcdata, int datasize, - const UncompressFlags& flags, int* width, int* height, - int* components, // Output only: useful with autodetect - int64* nwarn); - -// Version of Uncompress that allocates memory via a callback. The callback -// arguments are (width, height, components). If the size is known ahead of -// time this function can return an existing buffer; passing a callback allows -// the buffer to be shaped based on the JPEG header. The caller is responsible -// for freeing the memory *even along error paths*. -uint8* Uncompress(const void* srcdata, int datasize, - const UncompressFlags& flags, int64* nwarn, - std::function allocate_output); +// The function returns a shared pointer to the uncompressed data or a null pointer if +// there was an error. +std::unique_ptr Uncompress(const void* srcdata, int datasize, + const UncompressFlags& flags); // Read jpeg header and get image information. Returns true on success. // The width, height, and components points may be null. @@ -147,18 +129,6 @@ struct CompressFlags { int stride = 0; }; -// Compress some raw image given in srcdata, the data is a 2D array of size -// stride*height with one of the formats enumerated above. -// The encoded data is returned as a string. -// If not empty, XMP metadata can be embedded in the image header -// On error, returns the empty string (which is never a valid jpeg). -string Compress(const void* srcdata, int width, int height, - const CompressFlags& flags); - -// On error, returns false and sets output to empty. -bool Compress(const void* srcdata, int width, int height, - const CompressFlags& flags, string* output); - } // namespace jpeg } // namespace dali