fix(jpeg): validate metadata marker bounds

Signed-off-by: Vlad (Kuzmin) Erium <libalias@gmail.com>

Author: ssh4net

src/jpeg.imageio/jpeginput.cpp

@@ -52,6 +52,28 @@ OIIO_PLUGIN_EXPORTS_END
 static const uint8_t JPEG_MAGIC1 = 0xff;
 static const uint8_t JPEG_MAGIC2 = 0xd8;
 
+static const char exif_marker_prefix[] = "Exif\0";
+
+static const char icc_marker_prefix[] = "ICC_PROFILE";
+
+static bool
+is_exif_marker(jpeg_saved_marker_ptr marker)
+{
+    return marker->marker == (JPEG_APP0 + 1)
+           && marker->data_length >= sizeof(exif_marker_prefix)
+           && !memcmp(marker->data, exif_marker_prefix,
+                      sizeof(exif_marker_prefix));
+}
+
+static bool
+is_icc_profile_marker(jpeg_saved_marker_ptr marker)
+{
+    return marker->marker == (JPEG_APP0 + 2)
+           && marker->data_length >= ICC_HEADER_SIZE
+           && !memcmp(marker->data, icc_marker_prefix,
+                      sizeof(icc_marker_prefix));
+}
+
 
 // For explanations of the error handling, see the "example.c" in the
 // libjpeg distribution.
@@ -271,8 +293,7 @@ JpgInput::open(const std::string& name, ImageSpec& newspec)
         m_spec.attribute(JPEG_SUBSAMPLING_ATTR, subsampling);
 
     for (jpeg_saved_marker_ptr m = m_cinfo.marker_list; m; m = m->next) {
-        if (m->marker == (JPEG_APP0 + 1) && m->data_length >= 4
-            && !strncmp((const char*)m->data, "Exif", 4)) {
+        if (is_exif_marker(m)) {
             // The block starts with "Exif\0\0", so skip 6 bytes to get
             // to the start of the actual Exif data TIFF directory
             decode_exif(string_view((char*)m->data + 6, m->data_length - 6),
@@ -394,8 +415,7 @@ JpgInput::read_icc_profile(j_decompress_ptr cinfo, ImageSpec& spec)
     memset(marker_present, 0, (MAX_SEQ_NO + 1));
 
     for (jpeg_saved_marker_ptr m = cinfo->marker_list; m; m = m->next) {
-        if (m->marker == (JPEG_APP0 + 2)
-            && !strcmp((const char*)m->data, "ICC_PROFILE")) {
+        if (is_icc_profile_marker(m)) {
             if (num_markers == 0)
                 num_markers = GETJOCTET(m->data[13]);
             else if (num_markers != GETJOCTET(m->data[13]))
@@ -427,8 +447,7 @@ JpgInput::read_icc_profile(j_decompress_ptr cinfo, ImageSpec& spec)
 
     // and fill it in
     for (jpeg_saved_marker_ptr m = cinfo->marker_list; m; m = m->next) {
-        if (m->marker == (JPEG_APP0 + 2)
-            && !strcmp((const char*)m->data, "ICC_PROFILE")) {
+        if (is_icc_profile_marker(m)) {
             int seq_no = GETJOCTET(m->data[12]);
             if (data_offset[seq_no] + data_length[seq_no] > icc_buf.size()) {
                 errorfmt("Possible corrupt file, invalid ICC profile\n");

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

@@ -44,6 +44,11 @@ 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"
+short-exif-app1-len4-ok
+short-exif-app1-len5-ok
+short-icc-app2-len11-ok
+short-icc-app2-len12-ok
+short-icc-app2-len13-ok
 oiiotool ERROR: read : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
 Corrupted IPTC data
 Full command line was:

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

@@ -43,6 +43,11 @@ 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"
+short-exif-app1-len4-ok
+short-exif-app1-len5-ok
+short-icc-app2-len11-ok
+short-icc-app2-len12-ok
+short-icc-app2-len13-ok
 oiiotool ERROR: read : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
 Corrupted IPTC data
 Full command line was:

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

@@ -44,6 +44,11 @@ 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"
+short-exif-app1-len4-ok
+short-exif-app1-len5-ok
+short-icc-app2-len11-ok
+short-icc-app2-len12-ok
+short-icc-app2-len13-ok
 oiiotool ERROR: read : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
 Corrupted IPTC data
 Full command line was:

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

@@ -46,6 +46,11 @@ 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"
+short-exif-app1-len4-ok
+short-exif-app1-len5-ok
+short-icc-app2-len11-ok
+short-icc-app2-len12-ok
+short-icc-app2-len13-ok
 oiiotool ERROR: read : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
 Corrupted IPTC data
 Full command line was:

testsuite/jpeg-corrupt/ref/out.txt

@@ -44,6 +44,11 @@ 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"
+short-exif-app1-len4-ok
+short-exif-app1-len5-ok
+short-icc-app2-len11-ok
+short-icc-app2-len12-ok
+short-icc-app2-len13-ok
 oiiotool ERROR: read : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
 Corrupted IPTC data
 Full command line was:

testsuite/jpeg-corrupt/run.py

@@ -8,6 +8,9 @@
 failureok = 1
 redirect = ' >> out.txt 2>&1 '
 
+command += oiiotool("--create 1x1 3 -d uint8 -o base-short-marker.jpg")
+command += run_app(pythonbin + " src/make-short-marker-jpegs.py", silent=True)
+
 # This file has a corrupted Exif block in the metadata. It used to
 # crash on some platforms, on others would be caught by address sanitizer.
 # Fixed by #1635. This test serves to guard against regressions.
@@ -25,5 +28,13 @@
 # This file has a corrupted ICC profile block
 command += info_command ("src/corrupt-icc-4552.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.
+command += run_app(oiiotool("short-exif-app1-len4.jpg --echo short-exif-app1-len4-ok"))
+command += run_app(oiiotool("short-exif-app1-len5.jpg --echo short-exif-app1-len5-ok"))
+command += run_app(oiiotool("short-icc-app2-len11.jpg --echo short-icc-app2-len11-ok"))
+command += run_app(oiiotool("short-icc-app2-len12.jpg --echo short-icc-app2-len12-ok"))
+command += run_app(oiiotool("short-icc-app2-len13.jpg --echo short-icc-app2-len13-ok"))
+
 # This file had corrupted IPTC data
 command += oiiotool("-oiioattrib imageinput:strict 1 -info -v src/corrupt-iptc-8011.jpg")

testsuite/jpeg-corrupt/src/make-short-marker-jpegs.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python
+
+# Copyright Contributors to the OpenImageIO project.
+# SPDX-License-Identifier: Apache-2.0
+# https://github.com/AcademySoftwareFoundation/OpenImageIO
+
+from pathlib import Path
+
+
+APP1 = 0xE1
+APP2 = 0xE2
+BASE = Path("base-short-marker.jpg")
+
+
+def marker(marker_id, payload):
+    length = len(payload) + 2
+    return b"\xff" + bytes([marker_id]) + length.to_bytes(2, "big") + payload
+
+
+def write_with_marker(name, marker_id, payload):
+    data = BASE.read_bytes()
+    if data[:2] != b"\xff\xd8":
+        raise RuntimeError(f"{BASE} is not a JPEG stream")
+    Path(name).write_bytes(data[:2] + marker(marker_id, payload) + data[2:])
+
+
+write_with_marker("short-exif-app1-len4.jpg", APP1, b"Exif")
+write_with_marker("short-exif-app1-len5.jpg", APP1, b"Exif\0")
+write_with_marker("short-icc-app2-len11.jpg", APP2, b"ICC_PROFILE")
+write_with_marker("short-icc-app2-len12.jpg", APP2, b"ICC_PROFILE\0")
+write_with_marker("short-icc-app2-len13.jpg", APP2, b"ICC_PROFILE\0\1")