fix(icc): fix offsets, alignment-safe reads, 64-bit tag bounds

src/libOpenImageIO/icc.cpp

@@ -3,6 +3,7 @@
 // https://github.com/AcademySoftwareFoundation/OpenImageIO
 
 
+#include <cstring>
 #include <string>
 #include <unordered_map>
 
@@ -203,11 +204,16 @@ template<typename T>
 bool
 extract(cspan<uint8_t> iccdata, size_t& offset, T& result, std::string& error)
 {
-    if (offset + sizeof(T) > std::size(iccdata)) {
+    // Use subtraction rather than `offset + sizeof(T)` so a large offset can
+    // never overflow, and read via memcpy so an arbitrary (file-controlled)
+    // offset can never produce a misaligned load (UBSan alignment).
+    if (offset > std::size(iccdata)
+        || sizeof(T) > std::size(iccdata) - offset) {
         error = "ICC profile too small";
         return false;
     }
-    result = *(const T*)(iccdata.data() + offset);
+    // It could be unaligned, so use memcpy instead of `=`
+    memcpy(&result, iccdata.data() + offset, sizeof(T));
     offset += sizeof(T);
     if (littleendian())
         swap_endian(&result);
@@ -219,17 +225,31 @@ bool
 extract(cspan<uint8_t> iccdata, size_t& offset, ICCTag& result,
         std::string& error)
 {
-    if (offset + sizeof(result) > std::size(iccdata)) {
+    if (offset > std::size(iccdata)
+        || sizeof(result) > std::size(iccdata) - offset) {
         error = "ICC profile too small";
         return false;
     }
-    result = *(const ICCTag*)(iccdata.data() + offset);
+    // It could be unaligned, so use memcpy instead of `=`
+    memcpy(&result, iccdata.data() + offset, sizeof(ICCTag));
     offset += sizeof(ICCTag);
     if (littleendian())
         result.swap_endian();
     return true;
 }
 
+
+// Return true if the half-open byte range [offset, offset+len) lies entirely
+// within a buffer of `size` bytes. All arithmetic is done in 64 bits so a
+// pair of in-range uint32 fields (offset, len) can never overflow, and no
+// out-of-bounds pointer is ever formed to perform the check (which would be
+// undefined behavior, and unsafe on 32-bit platforms).
+inline bool
+range_in_bounds(uint64_t offset, uint64_t len, uint64_t size)
+{
+    return offset <= size && len <= size - offset;
+}
+
 }  // namespace
 
 OIIO_NAMESPACE_END
@@ -245,7 +265,8 @@ decode_icc_profile(cspan<uint8_t> iccdata, ImageSpec& spec, std::string& error)
         error = "ICC profile too small";
         return false;
     }
-    ICCHeader header = *(const ICCHeader*)iccdata.data();
+    ICCHeader header;
+    memcpy(&header, iccdata.data(), sizeof(ICCHeader));
     header.swap_endian();
     if (header.magic != 0x61637370) {
         error = "ICC profile has bad magic number";
@@ -307,8 +328,8 @@ 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 (!check_span(iccdata, iccdata.data() + tag.offset,
-                        std::max(4U, tag.size))) {
+        if (!range_in_bounds(tag.offset, std::max<uint64_t>(4, tag.size),
+                             iccdata.size())) {
             error = format(
                 "ICC profile tag {} appears to contain corrupted/invalid data",
                 signature);
@@ -377,18 +398,21 @@ 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)) {
+                    if (!range_in_bounds(uint64_t(tag.offset) + stroffset, len,
+                                         iccdata.size())) {
                         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
-                    std::u16string wstr((const char16_t*)start, len / 2);
+                    const uint8_t* start = iccdata.data() + tag.offset
+                                           + stroffset;
+                    // The actual data is UTF-16. Copy it out byte-wise (memcpy)
+                    // because `start` is at a file-controlled offset and may
+                    // not be 2-byte aligned (UBSan alignment).
+                    std::u16string wstr(len / 2, u'\0');
+                    memcpy(wstr.data(), start,
+                           size_t(len / 2) * sizeof(char16_t));
                     if (littleendian())
                         swap_endian((uint16_t*)wstr.data(), int(wstr.size()));
                     spec.attribute(tagname, Strutil::utf16_to_utf8(wstr));

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

@@ -64,6 +64,33 @@ src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
     ICCProfile:rendering_intent: "Unknown"
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-unaligned-mluc.jpg
+src/corrupt-icc-unaligned-mluc.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 0, 0, 0, 177, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [177 x uint8]
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 0
+    ICCProfile:color_space: ""
+    ICCProfile:creation_date: "00:00:00 00:00:00"
+    ICCProfile:creator_signature: "0"
+    ICCProfile:device_class: ""
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: ""
+    ICCProfile:profile_connection_space: ""
+    ICCProfile:profile_description: "Hi"
+    ICCProfile:profile_size: 177
+    ICCProfile:profile_version: "0.0.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-oversized.jpg
+src/corrupt-icc-oversized.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [132 x uint8]
+    oiio:ColorSpace: "srgb_rec709_scene"
 short-exif-app1-len4-ok
 short-exif-app1-len5-ok
 short-icc-app2-len11-ok

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

@@ -63,6 +63,33 @@ src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
     ICCProfile:rendering_intent: "Unknown"
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-unaligned-mluc.jpg
+src/corrupt-icc-unaligned-mluc.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 0, 0, 0, 177, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [177 x uint8]
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 0
+    ICCProfile:color_space: ""
+    ICCProfile:creation_date: "00:00:00 00:00:00"
+    ICCProfile:creator_signature: "0"
+    ICCProfile:device_class: ""
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: ""
+    ICCProfile:profile_connection_space: ""
+    ICCProfile:profile_description: "Hi"
+    ICCProfile:profile_size: 177
+    ICCProfile:profile_version: "0.0.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-oversized.jpg
+src/corrupt-icc-oversized.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [132 x uint8]
+    oiio:ColorSpace: "srgb_rec709_scene"
 short-exif-app1-len4-ok
 short-exif-app1-len5-ok
 short-icc-app2-len11-ok

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

@@ -64,6 +64,33 @@ src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
     ICCProfile:rendering_intent: "Unknown"
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-unaligned-mluc.jpg
+src/corrupt-icc-unaligned-mluc.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 0, 0, 0, 177, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [177 x uint8]
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 0
+    ICCProfile:color_space: ""
+    ICCProfile:creation_date: "00:00:00 00:00:00"
+    ICCProfile:creator_signature: "0"
+    ICCProfile:device_class: ""
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: ""
+    ICCProfile:profile_connection_space: ""
+    ICCProfile:profile_description: "Hi"
+    ICCProfile:profile_size: 177
+    ICCProfile:profile_version: "0.0.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-oversized.jpg
+src/corrupt-icc-oversized.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [132 x uint8]
+    oiio:ColorSpace: "srgb_rec709_scene"
 short-exif-app1-len4-ok
 short-exif-app1-len5-ok
 short-icc-app2-len11-ok

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

@@ -66,6 +66,33 @@ src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
     ICCProfile:rendering_intent: "Unknown"
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-unaligned-mluc.jpg
+src/corrupt-icc-unaligned-mluc.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 0, 0, 0, 177, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [177 x uint8]
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 0
+    ICCProfile:color_space: ""
+    ICCProfile:creation_date: "00:00:00 00:00:00"
+    ICCProfile:creator_signature: "0"
+    ICCProfile:device_class: ""
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: ""
+    ICCProfile:profile_connection_space: ""
+    ICCProfile:profile_description: "Hi"
+    ICCProfile:profile_size: 177
+    ICCProfile:profile_version: "0.0.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-oversized.jpg
+src/corrupt-icc-oversized.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [132 x uint8]
+    oiio:ColorSpace: "srgb_rec709_scene"
 short-exif-app1-len4-ok
 short-exif-app1-len5-ok
 short-icc-app2-len11-ok

testsuite/jpeg-corrupt/ref/out.txt

@@ -64,6 +64,33 @@ src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
     ICCProfile:rendering_intent: "Unknown"
     jpeg:subsampling: "4:2:0"
     oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-unaligned-mluc.jpg
+src/corrupt-icc-unaligned-mluc.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 0, 0, 0, 177, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [177 x uint8]
+    ICCProfile:attributes: "Reflective, Glossy, Positive, Color"
+    ICCProfile:cmm_type: 0
+    ICCProfile:color_space: ""
+    ICCProfile:creation_date: "00:00:00 00:00:00"
+    ICCProfile:creator_signature: "0"
+    ICCProfile:device_class: ""
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "0"
+    ICCProfile:model: "0"
+    ICCProfile:platform_signature: ""
+    ICCProfile:profile_connection_space: ""
+    ICCProfile:profile_description: "Hi"
+    ICCProfile:profile_size: 177
+    ICCProfile:profile_version: "0.0.0"
+    ICCProfile:rendering_intent: "Perceptual"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-icc-oversized.jpg
+src/corrupt-icc-oversized.jpg :    1 x    1, 1 channel, uint8 jpeg
+    SHA-1: 5BA93C9DB0CFF93F52B521D7420E43F6EDA2784F
+    channel list: Y
+    ICCProfile: 127, 255, 255, 255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ... [132 x uint8]
+    oiio:ColorSpace: "srgb_rec709_scene"
 short-exif-app1-len4-ok
 short-exif-app1-len5-ok
 short-icc-app2-len11-ok

testsuite/jpeg-corrupt/run.py

@@ -47,6 +47,20 @@
 # This file has a corrupted ICC profile block
 command += info_command ("src/corrupt-icc-4552.jpg", safematch=True)
 
+# This file's ICC profile places an 'mluc' (multi-localized unicode) tag at an
+# odd byte offset, with its English record pointing the UTF-16 string at
+# another odd offset. The shared ICC decoder read the record's 32-bit counts
+# and the 16-bit string code units directly through pointers at those
+# file-controlled offsets, so both loads were misaligned (UBSan `alignment`,
+# and a fault on strict-alignment CPUs). It now reads via memcpy and must
+# decode the profile_description cleanly.
+command += info_command ("src/corrupt-icc-unaligned-mluc.jpg", safematch=True)
+
+# This file's ICC header declares a profile_size far larger than the bytes that
+# follow. The decoder must reject it on the size-mismatch check with a bounded
+# read, never trusting the declared length.
+command += info_command ("src/corrupt-icc-oversized.jpg", safematch=True)
+
 # These files have short APP1/APP2 metadata marker payloads that used to be
 # read past their saved-marker buffers before being ignored. Use iconvert to
 # a null output to force a full input read.