From de90d1aec4d86112e2b86d1524b2330905131f6d Mon Sep 17 00:00:00 2001 From: arshsmith Date: Thu, 18 Jun 2026 21:56:14 +0530 Subject: [PATCH] Merge pull request #29322 from arshsmith:fix-exif-raw-profile-alloc Follow-up to the recent "reject under-length raw exif profile" change. The declared length in a PNG raw exif profile (tEXt/zTXt "Raw profile type exif" chunk) is fully attacker controlled. The < 6 guard stops the underflow, but a large positive value still passes it and reaches HexStringToBytes(), which does raw_data.resize(expected_length) before reading anything. So a few bytes in a text chunk can force a ~2GB allocation -> easy memory-amplification DoS while decoding an otherwise small PNG. The payload is hex encoded (two characters per byte), so a valid length can never exceed the profile text carrying it. Reject any declared length greater than profile_len. This bounds the allocation to the input size and doesn't reject any valid profile; the valid decode path is unchanged. --- modules/imgcodecs/src/exif.cpp | 88 +++++++++++++++++++--------- modules/imgcodecs/test/test_exif.cpp | 59 +++++++++++++++++++ 2 files changed, 118 insertions(+), 29 deletions(-) diff --git a/modules/imgcodecs/src/exif.cpp b/modules/imgcodecs/src/exif.cpp index 23b080b1e0..ab0ac787c5 100644 --- a/modules/imgcodecs/src/exif.cpp +++ b/modules/imgcodecs/src/exif.cpp @@ -54,34 +54,37 @@ namespace { namespace cv { -static std::string HexStringToBytes(const char* hexstring, size_t expected_length); +static std::string HexStringToBytes(const char* hexstring, size_t hexstring_len, size_t expected_length); -// Converts the NULL terminated 'hexstring' which contains 2-byte character -// representations of hex values to raw data. +// Converts the 'hexstring' (of at most 'hexstring_len' bytes) which contains +// 2-byte character representations of hex values to raw data. // 'hexstring' may contain values consisting of [A-F][a-f][0-9] in pairs, // e.g., 7af2..., separated by any number of newlines. // 'expected_length' is the anticipated processed size. // On success the raw buffer is returned with its length equivalent to -// 'expected_length'. NULL is returned if the processed length is less than -// 'expected_length' or any character aside from those above is encountered. -// The returned buffer must be freed by the caller. +// 'expected_length'. An empty buffer is returned if the processed length is +// less than 'expected_length' or any character aside from those above is +// encountered. All reads are bounded by 'hexstring_len'. static std::string HexStringToBytes(const char* hexstring, - size_t expected_length) { + size_t hexstring_len, size_t expected_length) { const char* src = hexstring; + const char* const src_end = hexstring + hexstring_len; size_t actual_length = 0; std::string raw_data; raw_data.resize(expected_length); char* dst = const_cast(raw_data.data()); - for (; actual_length < expected_length && *src != '\0'; ++src) { - char* end; + while (actual_length < expected_length && src < src_end) { + if (*src == '\n') { ++src; continue; } + if (src + 1 >= src_end) break; // need two characters to form a byte char val[3]; - if (*src == '\n') continue; - val[0] = *src++; - val[1] = *src; + char* end; + val[0] = src[0]; + val[1] = src[1]; val[2] = '\0'; *dst++ = static_cast(strtol(val, &end, 16)); if (end != val + 2) break; + src += 2; ++actual_length; } @@ -138,37 +141,64 @@ const std::vector& ExifReader::getData() const } bool ExifReader::processRawProfile(const char* profile, size_t profile_len) { - const char* src = profile; - char* end; - int expected_length; - if (profile == nullptr || profile_len == 0) return false; + const char* src = profile; + const char* const profile_end = profile + profile_len; + // ImageMagick formats 'raw profiles' as - // '\n\n(%8lu)\n\n'. + // '\n\n(%8lu)\n\n'. Every scan below is bounded by + // 'profile_end' so that a malformed (e.g. not NUL-terminated, or missing a + // newline) profile can never be read past its buffer. if (*src != '\n') { - CV_LOG_WARNING(NULL, cv::format("Malformed raw profile, expected '\\n' got '\\x%.2X'", *src)); + CV_LOG_WARNING(NULL, cv::format("Malformed raw profile, expected '\\n' got '\\x%.2X'", (unsigned char)*src)); return false; } ++src; - // skip the profile name and extract the length. - while (*src != '\0' && *src++ != '\n') {} - expected_length = static_cast(strtol(src, &end, 10)); - if (*end != '\n') { - CV_LOG_WARNING(NULL, cv::format("Malformed raw profile, expected '\\n' got '\\x%.2X'", *src)); + + // skip the profile name up to the next newline + while (src < profile_end && *src != '\n') ++src; + if (src >= profile_end) { // name not terminated within the buffer + CV_LOG_WARNING(NULL, "Malformed raw profile, profile name is not terminated"); + return false; + } + ++src; + + // the length token runs up to the next newline; parse it from a bounded, + // NUL-terminated copy (strtol handles the leading spaces of the %8lu field). + const char* len_begin = src; + while (src < profile_end && *src != '\n') ++src; + if (src >= profile_end) { // length not terminated within the buffer + CV_LOG_WARNING(NULL, "Malformed raw profile, length field is not terminated"); + return false; + } + const std::string len_str(len_begin, src); + ++src; // 'src' now points to the hex payload + + char* parse_end = nullptr; + const long declared_length = strtol(len_str.c_str(), &parse_end, 10); + if (parse_end == len_str.c_str() || *parse_end != '\0') { // not a clean decimal number + CV_LOG_WARNING(NULL, "Malformed raw profile, length field is not a valid number"); return false; } - ++end; // the payload starts with a 6-byte "Exif\0\0" header, so a shorter declared - // length underflows the size and pointer handed to parseExif() below - if (expected_length < 6) { + // length underflows the size and pointer handed to parseExif() below. it also + // cannot be larger than the profile text that carries it (two hex characters + // encode one byte), so reject an over-large value to avoid a huge speculative + // allocation in HexStringToBytes(). + if (declared_length < 6 || static_cast(declared_length) > profile_len) { + CV_LOG_WARNING(NULL, cv::format("Malformed raw profile, declared length %ld is out of range (profile size %llu)", + declared_length, (unsigned long long)profile_len)); return false; } + const size_t expected_length = static_cast(declared_length); - // 'end' now points to the profile payload. - std::string payload = HexStringToBytes(end, expected_length); - if (payload.size() == 0) return false; + std::string payload = HexStringToBytes(src, (size_t)(profile_end - src), expected_length); + if (payload.size() == 0) { // fewer hex bytes than the declared length + CV_LOG_WARNING(NULL, "Malformed raw profile, hex payload shorter than declared length"); + return false; + } return parseExif((unsigned char*)payload.c_str() + 6, expected_length - 6); } diff --git a/modules/imgcodecs/test/test_exif.cpp b/modules/imgcodecs/test/test_exif.cpp index ebc849e9e6..59a840a3f6 100644 --- a/modules/imgcodecs/test/test_exif.cpp +++ b/modules/imgcodecs/test/test_exif.cpp @@ -566,6 +566,65 @@ TEST(Imgcodecs_Png, Read_Exif_From_Text) EXPECT_EQ(read_metadata[0], exif_data); } +static uint32_t pngCrc32(const uchar* data, size_t len) +{ + uint32_t crc = 0xFFFFFFFFu; + for (size_t i = 0; i < len; i++) + { + crc ^= data[i]; + for (int k = 0; k < 8; k++) + crc = (crc & 1u) ? ((crc >> 1) ^ 0xEDB88320u) : (crc >> 1); + } + return crc ^ 0xFFFFFFFFu; +} + +static void pngAppendBE32(std::vector& v, uint32_t x) +{ + v.push_back((uchar)(x >> 24)); v.push_back((uchar)(x >> 16)); + v.push_back((uchar)(x >> 8)); v.push_back((uchar)x); +} + +// Regression: a PNG "Raw profile type exif" text chunk whose declared length is +// far larger than the payload it carries must be rejected (no multi-GB +// speculative allocation / no out-of-bounds read in ExifReader::processRawProfile) +// while the image itself still decodes. +TEST(Imgcodecs_Png, Read_Exif_From_Text_oversized_length_rejected) +{ + Mat img(8, 8, CV_8UC3, Scalar(10, 20, 30)); + std::vector png; + ASSERT_TRUE(imencode(".png", img, png)); + ASSERT_GT(png.size(), 33u); // 8-byte signature + 25-byte IHDR chunk + + const std::string keyword = "Raw profile type exif"; + const std::string profile = "\nexif\n999999999\n41414141\n"; // 9e8 declared, tiny payload + std::vector data(keyword.begin(), keyword.end()); + data.push_back(0); // keyword / text separator + data.insert(data.end(), profile.begin(), profile.end()); + + std::vector chunk; + pngAppendBE32(chunk, (uint32_t)data.size()); + const char type[4] = { 't', 'E', 'X', 't' }; + chunk.insert(chunk.end(), type, type + 4); + chunk.insert(chunk.end(), data.begin(), data.end()); + std::vector crc_input(type, type + 4); + crc_input.insert(crc_input.end(), data.begin(), data.end()); + pngAppendBE32(chunk, pngCrc32(crc_input.data(), crc_input.size())); + + // splice the tEXt chunk right after IHDR (valid placement for ancillary chunks) + png.insert(png.begin() + 33, chunk.begin(), chunk.end()); + + std::vector metadata_types; + std::vector > metadata; + Mat decoded; + ASSERT_NO_THROW(decoded = imdecodeWithMetadata(png, metadata_types, metadata, IMREAD_COLOR)); + ASSERT_FALSE(decoded.empty()); + EXPECT_EQ(decoded.rows, 8); + EXPECT_EQ(decoded.cols, 8); + // the malformed profile must not produce EXIF metadata + for (size_t i = 0; i < metadata_types.size(); i++) + EXPECT_NE(metadata_types[i], IMAGE_METADATA_EXIF); +} + static size_t locateString(const uchar* exif, size_t exif_size, const std::string& pattern) { size_t plen = pattern.size();