icc improvements, especially for PSD (4644)

The main thing here is that it turns out that when trying to parse the
ICC profiles from Photoshop PSD/PSB files, we weren't storing the
attributes in the correct way. (Everything else, we shoved in both the
"common" and "composite" imagespec, but we left ICC data out of the
composite spec.)

While I'm at it, I made two other ICC related changes:

* Among the several formats with ICC profiles, we were inconsistent
  about whether we report errors for corrupt ICC profiles and treat
  the whole image as broken, or just ignore the errors entirely.
  Fixed in various places to make a consistent policy of always
  checking the errors, and reporting/terminating if the
  "imageinput:strict" global attribute is true, allowing it to just
  ignore the broken ICC record if that attribute is false.

* Found a spot in JPEG handling of ICC where we were dangerously
  trusting of the sizes of things. Replace a risky memcpy with a safer
  spancpy that can't overrun the buffer bounds when copying.

Signed-off-by: Larry Gritz <[email protected]>

Author: lgritz

src/jpeg.imageio/jpeginput.cpp

@@ -427,16 +427,21 @@ JpgInput::read_icc_profile(j_decompress_ptr cinfo, ImageSpec& spec)
         if (m->marker == (JPEG_APP0 + 2)
             && !strcmp((const char*)m->data, "ICC_PROFILE")) {
             int seq_no = GETJOCTET(m->data[12]);
-            memcpy(icc_buf.data() + data_offset[seq_no],
-                   m->data + ICC_HEADER_SIZE, data_length[seq_no]);
+            if (data_offset[seq_no] + data_length[seq_no] > icc_buf.size()) {
+                errorfmt("Possible corrupt file, invalid ICC profile\n");
+                return false;
+            }
+            spancpy(make_span(icc_buf), data_offset[seq_no],
+                    make_cspan(m->data + ICC_HEADER_SIZE, data_length[seq_no]),
+                    0, data_length[seq_no]);
         }
     }
     spec.attribute("ICCProfile", TypeDesc(TypeDesc::UINT8, total_length),
                    icc_buf.data());
 
     std::string errormsg;
     bool ok = decode_icc_profile(icc_buf, spec, errormsg);
-    if (!ok) {
+    if (!ok && OIIO::get_int_attribute("imageinput:strict")) {
         errorfmt("Possible corrupt file, could not decode ICC profile: {}\n",
                  errormsg);
         return false;

src/jpeg2000.imageio/jpeg2000input.cpp

@@ -355,10 +355,10 @@ Jpeg2000Input::open(const std::string& name, ImageSpec& p_spec)
             cspan<uint8_t>((const uint8_t*)m_image->icc_profile_buf,
                            m_image->icc_profile_len),
             m_spec, errormsg);
-        if (!ok) {
-            // errorfmt("Could not decode ICC profile: {}\n", errormsg);
-            // return false;
-            // Nah, just skip an ICC specific error?
+        if (!ok && OIIO::get_int_attribute("imageinput:strict")) {
+            errorfmt("Possible corrupt file, could not decode ICC profile: {}\n",
+                     errormsg);
+            return false;
         }
     }
 

src/png.imageio/png_pvt.h

@@ -250,14 +250,12 @@ read_info(png_structp& sp, png_infop& ip, int& bit_depth, int& color_type,
                            TypeDesc(TypeDesc::UINT8, profile_length),
                            profile_data);
             std::string errormsg;
-            bool ok = decode_icc_profile(
-                cspan<uint8_t>((const uint8_t*)profile_data,
-                               span_size_t(profile_length)),
-                spec, errormsg);
-            if (!ok) {
-                // errorfmt("Could not decode ICC profile: {}\n", errormsg);
-                // return false;
-                // Nah, just skip an ICC specific error?
+            bool ok
+                = decode_icc_profile(make_cspan(profile_data, profile_length),
+                                     spec, errormsg);
+            if (!ok && OIIO::get_int_attribute("imageinput:strict")) {
+                errorfmt("Could not decode ICC profile: {}\n", errormsg);
+                return false;
             }
         }
     }

src/psd.imageio/psdinput.cpp

@@ -1245,8 +1245,15 @@ PSDInput::load_resource_1039(uint32_t length)
     TypeDesc type(TypeDesc::UINT8, length);
     common_attribute("ICCProfile", type, icc_buf.get());
     std::string errormsg;
-    decode_icc_profile(cspan<uint8_t>(icc_buf.get(), length), m_common_attribs,
-                       errormsg);
+    bool ok = decode_icc_profile(cspan<uint8_t>(icc_buf.get(), length),
+                                 m_common_attribs, errormsg)
+              && decode_icc_profile(cspan<uint8_t>(icc_buf.get(), length),
+                                    m_composite_attribs, errormsg);
+    if (!ok && OIIO::get_int_attribute("imageinput:strict")) {
+        errorfmt("Possible corrupt file, could not decode ICC profile: {}\n",
+                 errormsg);
+        return false;
+    }
     return true;
 }
 

src/tiff.imageio/tiffinput.cpp

@@ -209,7 +209,9 @@ class TIFFInput final : public ImageInput {
     // Read tags from the current directory of m_tif and fill out spec.
     // If read_meta is false, assume that m_spec already contains valid
     // metadata and should not be cleared or rewritten.
-    void readspec(bool read_meta = true);
+    // Return true if all is fine, false if something really bad happens,
+    // like we think the file is hopelessly corrupted.
+    bool readspec(bool read_meta = true);
 
     // Figure out all the photometric-related aspects of the header
     void readspec_photometric();
@@ -832,7 +834,8 @@ TIFFInput::seek_subimage(int subimage, int miplevel)
     m_next_scanline = 0;  // next scanline we'll read
     if (subimage == m_subimage || TIFFSetDirectory(m_tif, subimage)) {
         m_subimage = subimage;
-        readspec(read_meta);
+        if (!readspec(read_meta))
+            return false;
 
         char emsg[1024];
         if (m_use_rgba_interface && !TIFFRGBAImageOK(m_tif, emsg)) {
@@ -931,7 +934,7 @@ TIFFInput::spec_dimensions(int subimage, int miplevel)
 #define ICC_PROFILE_ATTR "ICCProfile"
 
 
-void
+bool
 TIFFInput::readspec(bool read_meta)
 {
     uint32_t width = 0, height = 0, depth = 0;
@@ -1201,7 +1204,7 @@ TIFFInput::readspec(bool read_meta)
     // assumed to be identical to what we already have in m_spec,
     // skip everything following.
     if (!read_meta)
-        return;
+        return true;
 
     short resunit = -1;
     TIFFGetField(m_tif, TIFFTAG_RESOLUTIONUNIT, &resunit);
@@ -1239,8 +1242,13 @@ TIFFInput::readspec(bool read_meta)
         m_spec.attribute(ICC_PROFILE_ATTR,
                          TypeDesc(TypeDesc::UINT8, icc_datasize), icc_buf);
         std::string errormsg;
-        decode_icc_profile(cspan<uint8_t>(icc_buf, icc_datasize), m_spec,
-                           errormsg);
+        bool ok = decode_icc_profile(cspan<uint8_t>(icc_buf, icc_datasize),
+                                     m_spec, errormsg);
+        if (!ok && OIIO::get_int_attribute("imageinput:strict")) {
+            errorfmt("Possible corrupt file, could not decode ICC profile: {}\n",
+                     errormsg);
+            return false;
+        }
     }
 
     // Search for an EXIF IFD in the TIFF file, and if found, rummage
@@ -1370,6 +1378,8 @@ TIFFInput::readspec(bool read_meta)
 
     if (m_testopenconfig)  // open-with-config debugging
         m_spec.attribute("oiio:DebugOpenConfig!", 42);
+
+    return true;
 }