From 03fd015e7716781fc97abeb82def05dd975a325f Mon Sep 17 00:00:00 2001 From: Shiho Midorikawa Date: Mon, 27 Feb 2023 03:26:10 +0900 Subject: [PATCH 1/4] Compute return buffer size at ExifManager::createJPEGMarker and make the buffer in that --- rtengine/imageio.cc | 6 +++--- rtexif/rtexif.cc | 24 +++++++++++++----------- rtexif/rtexif.h | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/rtengine/imageio.cc b/rtengine/imageio.cc index ce44d1ff4..fb7c9aee7 100644 --- a/rtengine/imageio.cc +++ b/rtengine/imageio.cc @@ -1027,7 +1027,7 @@ int ImageIO::savePNG (const Glib::ustring &fname, int bps) const #if defined(PNG_SKIP_sRGB_CHECK_PROFILE) && defined(PNG_SET_OPTION_SUPPORTED) png_set_option(png, PNG_SKIP_sRGB_CHECK_PROFILE, PNG_OPTION_ON); #endif - + png_infop info = png_create_info_struct(png); if (!info) { @@ -1227,12 +1227,12 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con jpeg_start_compress(&cinfo, TRUE); // buffer for exif and iptc markers - unsigned char* buffer = new unsigned char[165535]; //FIXME: no buffer size check so it can be overflowed in createJPEGMarker() for large tags, and then software will crash + unsigned char* buffer; unsigned int size; // assemble and write exif marker if (exifRoot) { - int size = rtexif::ExifManager::createJPEGMarker (exifRoot, *exifChange, cinfo.image_width, cinfo.image_height, buffer); + rtexif::ExifManager::createJPEGMarker (exifRoot, *exifChange, cinfo.image_width, cinfo.image_height, buffer, size); if (size > 0 && size < 65530) { jpeg_write_marker(&cinfo, JPEG_APP0 + 1, buffer, size); diff --git a/rtexif/rtexif.cc b/rtexif/rtexif.cc index a7125fb9a..a48db5229 100644 --- a/rtexif/rtexif.cc +++ b/rtexif/rtexif.cc @@ -3243,24 +3243,17 @@ std::vector ExifManager::getDefaultTIFFTags (TagDirectory* forthis) -int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char* buffer) +int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize) { // write tiff header - int offs = 6; - memcpy (buffer, "Exif\0\0", 6); + int offs = 6; // "Exif\0\0" ByteOrder order = INTEL; if (root) { order = root->getOrder (); } - sset2 ((unsigned short)order, buffer + offs, order); - offs += 2; - sset2 (42, buffer + offs, order); - offs += 2; - sset4 (8, buffer + offs, order); - TagDirectory* cl; if (root) { @@ -3322,11 +3315,20 @@ int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::pro } cl->sort (); - int size = cl->write (8, buffer + 6); + bufferSize = cl->calculateSize() + 8 + 6; + buffer = new unsigned char[bufferSize]; // this has to be deleted in caller + memcpy (buffer, "Exif\0\0", 6); + sset2 ((unsigned short)order, buffer + offs, order); + offs += 2; + sset2 (42, buffer + offs, order); + offs += 2; + sset4 (8, buffer + offs, order); + + int endOffs = cl->write (8, buffer + 6); delete cl; - return size + 6; + return endOffs; } int ExifManager::createPNGMarker(const TagDirectory* root, const rtengine::procparams::ExifPairs &changeList, int W, int H, int bps, const char* iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize) diff --git a/rtexif/rtexif.h b/rtexif/rtexif.h index 7b2f8ad23..beb21131a 100644 --- a/rtexif/rtexif.h +++ b/rtexif/rtexif.h @@ -379,7 +379,7 @@ public: /// @param forthis The byte order will be taken from the given directory. /// @return The ownership of the return tags is passed to the caller. static std::vector getDefaultTIFFTags (TagDirectory* forthis); - static int createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char* buffer); + static int createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize); static int createTIFFHeader (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, int bps, const char* profiledata, int profilelen, const char* iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize); static int createPNGMarker(const TagDirectory *root, const rtengine::procparams::ExifPairs &changeList, int W, int H, int bps, const char *iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize); }; From 62c1f34c4f68beeb1338609ba6065bb459baab0e Mon Sep 17 00:00:00 2001 From: Shiho Midorikawa Date: Mon, 27 Feb 2023 03:43:25 +0900 Subject: [PATCH 2/4] Separate IPTC/EXIF buffers --- rtengine/imageio.cc | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/rtengine/imageio.cc b/rtengine/imageio.cc index fb7c9aee7..16f6a7246 100644 --- a/rtengine/imageio.cc +++ b/rtengine/imageio.cc @@ -1226,19 +1226,27 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con jpeg_start_compress(&cinfo, TRUE); - // buffer for exif and iptc markers - unsigned char* buffer; - unsigned int size; + // buffer for exif marker + unsigned char* exifBuffer = nullptr; + unsigned int exifBufferSize = 0; // assemble and write exif marker if (exifRoot) { - rtexif::ExifManager::createJPEGMarker (exifRoot, *exifChange, cinfo.image_width, cinfo.image_height, buffer, size); + rtexif::ExifManager::createJPEGMarker (exifRoot, *exifChange, cinfo.image_width, cinfo.image_height, exifBuffer, exifBufferSize); - if (size > 0 && size < 65530) { - jpeg_write_marker(&cinfo, JPEG_APP0 + 1, buffer, size); + if (exifBufferSize > 0 && exifBufferSize < 65530) { + jpeg_write_marker(&cinfo, JPEG_APP0 + 1, exifBuffer, exifBufferSize); } + } + if (exifBuffer != nullptr) { + delete [] exifBuffer; + } + + // buffer for iptc marker + unsigned char* iptcBuffer = new unsigned char[65535]; + // assemble and write iptc marker if (iptc) { unsigned char* iptcdata; @@ -1254,7 +1262,7 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con int bytes = 0; - if (!error && (bytes = iptc_jpeg_ps3_save_iptc (nullptr, 0, iptcdata, size, buffer, 65532)) < 0) { + if (!error && (bytes = iptc_jpeg_ps3_save_iptc (nullptr, 0, iptcdata, size, iptcBuffer, 65532)) < 0) { error = true; } @@ -1263,11 +1271,11 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con } if (!error) { - jpeg_write_marker(&cinfo, JPEG_APP0 + 13, buffer, bytes); + jpeg_write_marker(&cinfo, JPEG_APP0 + 13, iptcBuffer, bytes); } } - delete [] buffer; + delete [] iptcBuffer; // write icc profile to the output if (profileData) { From f759cff2fe4f6011c38039c6b5c077bbea69b79d Mon Sep 17 00:00:00 2001 From: Shiho Midorikawa Date: Mon, 27 Feb 2023 14:27:47 +0900 Subject: [PATCH 3/4] Fix separation mistake --- rtengine/imageio.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rtengine/imageio.cc b/rtengine/imageio.cc index 16f6a7246..23b0fafec 100644 --- a/rtengine/imageio.cc +++ b/rtengine/imageio.cc @@ -1250,9 +1250,10 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con // assemble and write iptc marker if (iptc) { unsigned char* iptcdata; + unsigned int iptcSize; bool error = false; - if (iptc_data_save (iptc, &iptcdata, &size)) { + if (iptc_data_save (iptc, &iptcdata, &iptcSize)) { if (iptcdata) { iptc_data_free_buf (iptc, iptcdata); } @@ -1262,7 +1263,7 @@ int ImageIO::saveJPEG (const Glib::ustring &fname, int quality, int subSamp) con int bytes = 0; - if (!error && (bytes = iptc_jpeg_ps3_save_iptc (nullptr, 0, iptcdata, size, iptcBuffer, 65532)) < 0) { + if (!error && (bytes = iptc_jpeg_ps3_save_iptc (nullptr, 0, iptcdata, iptcSize, iptcBuffer, 65532)) < 0) { error = true; } From fcbe2182fde376521bc10ea76da710f334e4025e Mon Sep 17 00:00:00 2001 From: Shiho Midorikawa Date: Thu, 25 May 2023 03:13:07 +0900 Subject: [PATCH 4/4] Remove return value from ExifManager::createJPEGMarker --- rtexif/rtexif.cc | 6 ++---- rtexif/rtexif.h | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/rtexif/rtexif.cc b/rtexif/rtexif.cc index a48db5229..111f11668 100644 --- a/rtexif/rtexif.cc +++ b/rtexif/rtexif.cc @@ -3243,7 +3243,7 @@ std::vector ExifManager::getDefaultTIFFTags (TagDirectory* forthis) -int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize) +void ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize) { // write tiff header @@ -3324,11 +3324,9 @@ int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::pro offs += 2; sset4 (8, buffer + offs, order); - int endOffs = cl->write (8, buffer + 6); + cl->write (8, buffer + 6); delete cl; - - return endOffs; } int ExifManager::createPNGMarker(const TagDirectory* root, const rtengine::procparams::ExifPairs &changeList, int W, int H, int bps, const char* iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize) diff --git a/rtexif/rtexif.h b/rtexif/rtexif.h index beb21131a..0b44bc47a 100644 --- a/rtexif/rtexif.h +++ b/rtexif/rtexif.h @@ -379,7 +379,7 @@ public: /// @param forthis The byte order will be taken from the given directory. /// @return The ownership of the return tags is passed to the caller. static std::vector getDefaultTIFFTags (TagDirectory* forthis); - static int createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize); + static void createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char *&buffer, unsigned &bufferSize); static int createTIFFHeader (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, int bps, const char* profiledata, int profilelen, const char* iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize); static int createPNGMarker(const TagDirectory *root, const rtengine::procparams::ExifPairs &changeList, int W, int H, int bps, const char *iptcdata, int iptclen, unsigned char *&buffer, unsigned &bufferSize); };