Merge pull request #6691 from elliptic-shiho/fix-jpeg-exif-buffer-size

Fix the buffer-size computation flow of exif marker of JPEG

rtengine/imageio.cc

@@ -1033,7 +1033,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) {
@@ -1232,25 +1232,34 @@ 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 int size;
+    // buffer for exif marker
+    unsigned char* exifBuffer = nullptr;
+    unsigned int exifBufferSize = 0;
 
     // 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, 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;
+        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);
             }
@@ -1260,7 +1269,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, iptcSize, iptcBuffer, 65532)) < 0) {
             error = true;
         }
 
@@ -1269,11 +1278,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) {

rtexif/rtexif.cc

@@ -3247,24 +3247,17 @@ std::vector<Tag*> ExifManager::getDefaultTIFFTags (TagDirectory* forthis)
 
 
 
-int ExifManager::createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char* buffer)
+void 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) {
@@ -3326,11 +3319,18 @@ 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);
+
+    cl->write (8, buffer + 6);
 
     delete cl;
-
-    return size + 6;
 }
 
 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)

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<Tag*> getDefaultTIFFTags (TagDirectory* forthis);
-    static int    createJPEGMarker (const TagDirectory* root, const rtengine::procparams::ExifPairs& changeList, int W, int H, unsigned char* buffer);
+    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);
 };