diff --git a/lib/src/jpegrutils.cpp b/lib/src/jpegrutils.cpp index 3f064ade..c3f9293b 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,26 @@ 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; + // 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; 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;