fix: Detect invalid ICC profile tags (#4557)

Affects jpeg, tiff, and other formats with embedded ICC profiles.
Corrupted/invalid tag offset and/or length values were causing us to
touch memory outside the memory allocated for the profile.

Fixes #4551

Not sure if this is wise or not, but for JPEGs, I also enabled some
previously commented-out code that has the effect of treating a file
with a verifiably corrupted/nonsensical ICC profile block as a failed
file. If people ever complain about this in the future, maybe we'll come
back and add some kind of global option that lets the app express how
tolerant OIIO readers should be about corruptions in files -- do you
assume that the whole file is bogus or malicious upon discovering the
first invalidity, or do you press on and hope you can get something else
useful from the pixels? Let's be cautious for now.

---------

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

Author: lgritz

src/jpeg.imageio/jpeginput.cpp

@@ -437,9 +437,9 @@ JpgInput::read_icc_profile(j_decompress_ptr cinfo, ImageSpec& spec)
     std::string errormsg;
     bool ok = decode_icc_profile(icc_buf, spec, errormsg);
     if (!ok) {
-        // errorfmt("Could not decode ICC profile: {}\n", errormsg);
-        // return false;
-        // Nah, just skip an ICC specific error?
+        errorfmt("Possible corrupt file, could not decode ICC profile: {}\n",
+                 errormsg);
+        return false;
     }
 
     return true;

src/libOpenImageIO/icc.cpp

@@ -304,11 +304,13 @@ decode_icc_profile(cspan<uint8_t> iccdata, ImageSpec& spec, std::string& error)
         if (!extract(iccdata, offset, tag, error))
             return false;
         string_view signature(tag.signature, 4);
-        if (tag.offset + tag.size > iccdata.size()) {
-            error = format("ICC profile tag {} extends past end", signature);
-            return false;
+        if (!check_span(iccdata, iccdata.data() + tag.offset,
+                        std::max(4U, tag.size))) {
+            error = format(
+                "ICC profile tag {} appears to contain corrupted/invalid data",
+                signature);
+            return false;  // Non-sensical: tag extends beyond icc data block
         }
-
         string_view typesignature((const char*)iccdata.data() + tag.offset, 4);
         // Strutil::print("   tag {} type {} offset {} size {}\n", signature,
         //                typesignature, tag.offset, tag.size);
@@ -360,6 +362,14 @@ decode_icc_profile(cspan<uint8_t> iccdata, ImageSpec& spec, std::string& error)
                     // Strutil::print(
                     //     "eng len={} stfoffset={} ({:x}) wcharsize={}\n", len,
                     //     stroffset, tag.offset + stroffset, sizeof(wchar_t));
+                    if (!check_span(iccdata,
+                                    iccdata.data() + tag.offset + stroffset,
+                                    len)) {
+                        error = format(
+                            "ICC profile tag {} appears to contain corrupted/invalid data",
+                            signature);
+                        return false;  // Non-sensical: tag extends beyond icc data block
+                    }
                     const char* start = (const char*)iccdata.data() + tag.offset
                                         + stroffset;
                     // The actual data is UTF-16

testsuite/jpeg-corrupt/ref/out-alt.txt

@@ -18,3 +18,5 @@ src/corrupt-exif-1626.jpg :  256 x  256, 3 channel, uint8 jpeg
     YResolution: 300
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "sRGB"
+corrupt-icc-4551.jpg
+DCT coefficient (lossy) or spatial difference (lossless) out of range

testsuite/jpeg-corrupt/ref/out-alt2.txt

@@ -18,3 +18,5 @@ src/corrupt-exif-1626.jpg :  256 x  256, 3 channel, uint8 jpeg
     YResolution: 300
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "sRGB"
+corrupt-icc-4551.jpg
+DCT coefficient (lossy) or spatial difference (lossless) out of range

testsuite/jpeg-corrupt/ref/out-alt3.txt

@@ -0,0 +1,21 @@
+Reading src/corrupt-exif.jpg
+src/corrupt-exif.jpg :  256 x  256, 3 channel, uint8 jpeg
+    SHA-1: 3EA7BAEB0871E9B9BAADF286521423199ACA2401
+    channel list: R, G, B
+    Orientation: 1 (normal)
+    ResolutionUnit: "in"
+    XResolution: 300
+    YResolution: 300
+    jpeg:subsampling: "4:2:0"
+    oiio:ColorSpace: "sRGB"
+Reading src/corrupt-exif-1626.jpg
+src/corrupt-exif-1626.jpg :  256 x  256, 3 channel, uint8 jpeg
+    SHA-1: 6CBB7DB602A67DB300AB28842F20F6DCA02B82B1
+    channel list: R, G, B
+    Orientation: 1 (normal)
+    ResolutionUnit: "in"
+    XResolution: 300
+    YResolution: 300
+    jpeg:subsampling: "4:2:0"
+    oiio:ColorSpace: "sRGB"
+corrupt-icc-4551.jpg

testsuite/jpeg-corrupt/ref/out.txt

@@ -18,3 +18,5 @@ src/corrupt-exif-1626.jpg :  256 x  256, 3 channel, uint8 jpeg
     YResolution: 300
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "sRGB"
+corrupt-icc-4551.jpg
+DCT coefficient (lossy) or spatial difference (lossless) out of range

testsuite/jpeg-corrupt/run.py

@@ -17,3 +17,7 @@
 # nonsensical length, that before being fixed, caused a buffer overrun.
 command += info_command ("src/corrupt-exif-1626.jpg", safematch=True)
 
+# This file has a corrupted ICC profile block that has tags that say they
+# extend beyond the boundaries of the ICC block itself.
+command += run_app (oiiotool("--echo corrupt-icc-4551.jpg"))
+command += run_app (oiio_app("iconvert") + " src/corrupt-icc-4551.jpg out-4551.jpg")