From b9ad439b16fffccfef0e6c403943968f52f84a68 Mon Sep 17 00:00:00 2001 From: Alejandro Pacheco Date: Wed, 14 Oct 2026 09:17:18 +0200 Subject: [PATCH 1/2] jpegrutils: check EXIF offsets before adding them in getExifAppleHeadroom getExifAppleHeadroom adds offsets read from the EXIF data to the position of the TIFF header or of the Apple Maker Notes and compares the sum with the buffer size afterwards. On 32-bit targets the sum can wrap, so the Maker Notes check passes for an offset close to UINT32_MAX and memcmp reads before the start of the EXIF buffer. Resolve every offset taken from the EXIF data through one helper that rejects positions past the end of the buffer without forming the sum first, and compare the Maker Notes header through a bounds-checked helper. This replaces the separate underflow and overflow guards of the tag 33/48 path. Valid EXIF parses to the same positions as before. Add tests for reading the headroom from Apple Maker Notes and for rejecting out-of-range IFD, Maker Notes and value offsets. --- lib/src/jpegrutils.cpp | 49 +++++++++++------- tests/jpegr_test.cpp | 115 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 19 deletions(-) diff --git a/lib/src/jpegrutils.cpp b/lib/src/jpegrutils.cpp index 3f064ade..8e87e338 100644 --- a/lib/src/jpegrutils.cpp +++ b/lib/src/jpegrutils.cpp @@ -509,6 +509,21 @@ static bool readS32(const uint8_t* data, size_t size, int32_t* value, size_t* of return true; } +// Resolves an offset stored in the EXIF data, relative to base, to an offset in +// the buffer. Fails if the result lies past the end of the buffer. Every offset +// read from the EXIF data must go through this, so that the sum never wraps. +static bool resolveOffset(size_t base, uint32_t relOffset, size_t size, size_t* offset) { + if (base > size || relOffset > size - base) return false; + *offset = base + relOffset; + return true; +} + +static bool matchBytes(const uint8_t* data, size_t size, size_t offset, const uint8_t* expected, + size_t expectedSize) { + if (offset > size || size - offset < expectedSize) return false; + return !memcmp(&data[offset], expected, expectedSize); +} + static bool getExifAppleHeadroom(const uint8_t* exif, size_t size, float* altHeadroom) { *altHeadroom = 0.0f; size_t offset = 0; @@ -554,7 +569,9 @@ static bool getExifAppleHeadroom(const uint8_t* exif, size_t size, float* altHea const size_t tiffHeaderOffset = offset - 8; while (offsetToIfd != 0 && numIfds++ < maxIfds) { - offset = tiffHeaderOffset + offsetToIfd; + size_t ifdOffset; + if (!resolveOffset(tiffHeaderOffset, offsetToIfd, size, &ifdOffset)) return false; + offset = ifdOffset; bool offsetToNextIfdAlreadySet = false; uint16_t fieldCount; @@ -575,30 +592,24 @@ static bool getExifAppleHeadroom(const uint8_t* exif, size_t size, float* altHea offsetToNextIfdAlreadySet = true; break; } else if (tagId == 0x927c) { // Maker Notes - uint32_t makerNotesOffset = tagData; - if (tiffHeaderOffset + makerNotesOffset + appleMakerNotesHeaderSize <= size && - !memcmp(&exif[tiffHeaderOffset + makerNotesOffset], appleMakerNotesHeader, - appleMakerNotesHeaderSize)) { - offsetToIfd = makerNotesOffset + (uint32_t)appleMakerNotesHeaderSize; + size_t makerNotesOffset; + if (resolveOffset(tiffHeaderOffset, tagData, size, &makerNotesOffset) && + matchBytes(exif, size, makerNotesOffset, appleMakerNotesHeader, + appleMakerNotesHeaderSize)) { + offsetToIfd = tagData + (uint32_t)appleMakerNotesHeaderSize; inAppleMakerNotes = true; offsetToNextIfdAlreadySet = true; isBigEndian = true; // Apple Maker Notes are always big endian. break; } } else if (inAppleMakerNotes && (tagId == 33 || tagId == 48) && dataFormat == 10) { - // Guard against size_t underflow before the subtraction. If - // (tiffHeaderOffset + offsetToIfd) is smaller than the Apple Maker - // Notes header size, the unsigned arithmetic would wrap around to a - // value close to SIZE_MAX, and the subsequent readU32/readS32 bounds - // check (*offset + 4 > size) would also overflow and silently pass, - // resulting in an out-of-bounds read. - if (tiffHeaderOffset + offsetToIfd < appleMakerNotesHeaderSize) return false; - size_t tmpOffset = - tiffHeaderOffset + offsetToIfd - appleMakerNotesHeaderSize; - // Guard against size_t overflow when adding the attacker-controlled - // tagData to the computed base offset. - if (tmpOffset > SIZE_MAX - (size_t)tagData) return false; - tmpOffset += (size_t)tagData; + // Offsets in the Apple Maker Notes are relative to the Maker Notes + // header, which precedes their first IFD. + if (ifdOffset < appleMakerNotesHeaderSize) return false; + size_t tmpOffset; + if (!resolveOffset(ifdOffset - appleMakerNotesHeaderSize, tagData, size, &tmpOffset)) { + return false; + } int32_t numerator; uint32_t denominator; if (!readS32(exif, size, &numerator, &tmpOffset, isBigEndian)) return false; diff --git a/tests/jpegr_test.cpp b/tests/jpegr_test.cpp index 73d75623..51b53e6e 100644 --- a/tests/jpegr_test.cpp +++ b/tests/jpegr_test.cpp @@ -1712,6 +1712,121 @@ TEST(JpegRTest, writeXmpThenRead) { EXPECT_TRUE(metadata_read.use_base_cg); } +// Apple XMP without HDRGainMapHeadroom, so that the headroom is read from EXIF. +static std::vector appleXmpWithoutHeadroom() { + const std::string nameSpace = "http://ns.adobe.com/xap/1.0/\0"; + const size_t nameSpaceLength = nameSpace.size() + 1; + const std::string xmp = + "" + "65536" + ""; + std::vector xmpData; + xmpData.insert(xmpData.end(), reinterpret_cast(nameSpace.c_str()), + reinterpret_cast(nameSpace.c_str()) + nameSpaceLength); + xmpData.insert(xmpData.end(), xmp.begin(), xmp.end()); + return xmpData; +} + +static void putU16(std::vector& v, size_t pos, uint16_t x, bool bigEndian) { + v[pos + (bigEndian ? 0 : 1)] = x >> 8; + v[pos + (bigEndian ? 1 : 0)] = x & 0xff; +} + +static void putU32(std::vector& v, size_t pos, uint32_t x, bool bigEndian) { + putU16(v, pos + (bigEndian ? 0 : 2), x >> 16, bigEndian); + putU16(v, pos + (bigEndian ? 2 : 0), x & 0xffff, bigEndian); +} + +// EXIF with little endian TIFF data: IFD0 holds an Exif sub-IFD pointer, the sub-IFD +// holds the Maker Notes, which are Apple Maker Notes with tags 33 and 48 (rationals). +// Offsets of the IFDs and rationals are parameters, relative to the TIFF header and the +// Maker Notes respectively. +struct AppleExifOffsets { + uint32_t ifd0 = 8; + uint32_t exifIfd = 26; + uint32_t makerNotes = 44; + uint32_t tag33 = 46; + uint32_t tag48 = 54; +}; + +static std::vector appleExif(const AppleExifOffsets& o) { + const size_t kTiff = 6; + std::vector exif(kTiff + 44 + 62, 0); + memcpy(exif.data(), "Exif\0\0II*\0", 10); + putU32(exif, kTiff + 4, o.ifd0, false); + // IFD0 at TIFF + 8: one entry, the Exif sub-IFD pointer. + putU16(exif, kTiff + 8, 1, false); + putU16(exif, kTiff + 10, 0x8769, false); + putU16(exif, kTiff + 12, 4, false); + putU32(exif, kTiff + 14, 1, false); + putU32(exif, kTiff + 18, o.exifIfd, false); + // Exif sub-IFD at TIFF + 26: one entry, the Maker Notes. + putU16(exif, kTiff + 26, 1, false); + putU16(exif, kTiff + 28, 0x927c, false); + putU16(exif, kTiff + 30, 7, false); + putU32(exif, kTiff + 32, 62, false); + putU32(exif, kTiff + 36, o.makerNotes, false); + // Apple Maker Notes at TIFF + 44, big endian, offsets relative to their start. + const size_t kMaker = kTiff + 44; + const uint8_t kHeader[] = {'A', 'p', 'p', 'l', 'e', ' ', 'i', 'O', 'S', 0, 0, 1, 'M', 'M'}; + memcpy(&exif[kMaker], kHeader, sizeof(kHeader)); + putU16(exif, kMaker + 14, 2, true); + putU16(exif, kMaker + 16, 33, true); + putU16(exif, kMaker + 18, 10, true); + putU32(exif, kMaker + 20, 1, true); + putU32(exif, kMaker + 24, o.tag33, true); + putU16(exif, kMaker + 28, 48, true); + putU16(exif, kMaker + 30, 10, true); + putU32(exif, kMaker + 32, 1, true); + putU32(exif, kMaker + 36, o.tag48, true); + // Rationals: tag 33 = 1/2, tag 48 = 5/1000. + putU32(exif, kMaker + 46, 1, true); + putU32(exif, kMaker + 50, 2, true); + putU32(exif, kMaker + 54, 5, true); + putU32(exif, kMaker + 58, 1000, true); + return exif; +} + +TEST(JpegRTest, readAppleHeadroomFromExif) { + std::vector xmpData = appleXmpWithoutHeadroom(); + std::vector exif = appleExif(AppleExifOffsets()); + uhdr_gainmap_metadata_ext_t metadata; + ASSERT_EQ(getMetadataFromXMP(xmpData.data(), xmpData.size(), exif.data(), exif.size(), &metadata) + .error_code, + UHDR_CODEC_OK); + // maker33 < 1 and maker48 <= 0.01: stops = -20 * maker48 + 1.8. + EXPECT_FLOAT_EQ(metadata.hdr_capacity_max, (float)pow(2.0, 1.7)); + EXPECT_FLOAT_EQ(metadata.max_content_boost[0], (float)pow(2.0, 1.7)); +} + +// Every offset read from the EXIF data must be rejected when it points past the end of the +// data, including values that would wrap around when added to the position of the TIFF +// header or of the Maker Notes. +TEST(JpegRTest, rejectAppleExifOffsetsPastEnd) { + std::vector xmpData = appleXmpWithoutHeadroom(); + const size_t exifSize = appleExif(AppleExifOffsets()).size(); + const uint32_t kBadOffsets[] = {(uint32_t)exifSize, (uint32_t)exifSize - 1, + 0xffffffffu, 0xfffffffau, + 0xfffffff2u, 0xffffffe0u, + 0xffffffd2u, 0x80000000u}; + for (uint32_t bad : kBadOffsets) { + for (int field = 0; field < 5; ++field) { + AppleExifOffsets o; + uint32_t* fields[] = {&o.ifd0, &o.exifIfd, &o.makerNotes, &o.tag33, &o.tag48}; + *fields[field] = bad; + std::vector exif = appleExif(o); + uhdr_gainmap_metadata_ext_t metadata; + EXPECT_EQ( + getMetadataFromXMP(xmpData.data(), xmpData.size(), exif.data(), exif.size(), &metadata) + .error_code, + UHDR_CODEC_ERROR) + << "field " << field << " offset " << bad; + } + } +} + TEST(JpegRTest, decodeApple) { JpegR decoder; uhdr_compressed_image_t uhdrCompressedImg; From 35e5ef37c1e440f707646e75ea72d195e3fd87f0 Mon Sep 17 00:00:00 2001 From: Alejandro Pacheco Date: Wed, 14 Oct 2026 10:06:12 +0200 Subject: [PATCH 2/2] jpegrutils: keep the Apple Maker Notes IFD offset within 32 bits The offset of the first IFD after the Maker Notes header is stored in a uint32_t. Reject a Maker Notes offset that would wrap it, instead of relying on the EXIF size staying below 4 GiB. --- lib/src/jpegrutils.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/src/jpegrutils.cpp b/lib/src/jpegrutils.cpp index 8e87e338..c3f9293b 100644 --- a/lib/src/jpegrutils.cpp +++ b/lib/src/jpegrutils.cpp @@ -593,7 +593,9 @@ static bool getExifAppleHeadroom(const uint8_t* exif, size_t size, float* altHea break; } else if (tagId == 0x927c) { // Maker Notes size_t makerNotesOffset; - if (resolveOffset(tiffHeaderOffset, tagData, size, &makerNotesOffset) && + // The first IFD follows the header; its offset must fit in 32 bits too. + if (tagData <= UINT32_MAX - appleMakerNotesHeaderSize && + resolveOffset(tiffHeaderOffset, tagData, size, &makerNotesOffset) && matchBytes(exif, size, makerNotesOffset, appleMakerNotesHeader, appleMakerNotesHeaderSize)) { offsetToIfd = tagData + (uint32_t)appleMakerNotesHeaderSize;