From dbf90b976fff05d66ebfb2e0e05157355a86fd1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Sat, 7 Jul 2018 09:27:59 +0200 Subject: [PATCH] Fix overread in memcmp in PngImage::doWriteMetadata() memcmp() compares the read data from key with the provided string, but when key.pData_ is shorter than the provided length, then memcmp can read beyond the bounds of key.pData_ => add custom compare function, which ensures that we never read more than key.size_ --- src/pngimage.cpp | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/pngimage.cpp b/src/pngimage.cpp index 984f7532..ffd04ce5 100644 --- a/src/pngimage.cpp +++ b/src/pngimage.cpp @@ -1,6 +1,6 @@ // ***************************************************************** -*- C++ -*- /* - * Copyright (C) 2004-2017 Andreas Huggel + * Copyright (C) 2004-2018 Exiv2 authors * * This program is part of the Exiv2 distribution. * @@ -57,6 +57,16 @@ const unsigned char pngBlank[] = { 0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a,0x00, 0x45,0x4e,0x44,0xae,0x42,0x60,0x82 }; +namespace +{ + inline bool compare(const char* str, const Exiv2::DataBuf& buf, size_t length) + { + // str & length should compile time constants => only running this in DEBUG mode is ok + assert(strlen(str) <= length); + return memcmp(str, buf.pData_, std::min(static_cast(length), buf.size_)) == 0; + } +} // namespace + // ***************************************************************************** // class member definitions namespace Exiv2 { @@ -688,14 +698,14 @@ namespace Exiv2 { !memcmp(cheaderBuf.pData_ + 4, "iCCP", 4)) { DataBuf key = PngChunk::keyTXTChunk(chunkBuf, true); - if (memcmp("Raw profile type exif", key.pData_, 21) == 0 || - memcmp("Raw profile type APP1", key.pData_, 21) == 0 || - memcmp("Raw profile type iptc", key.pData_, 21) == 0 || - memcmp("Raw profile type xmp", key.pData_, 20) == 0 || - memcmp("XML:com.adobe.xmp", key.pData_, 17) == 0 || - memcmp("icc", key.pData_, 3) == 0 || // see test/data/imagemagick.png - memcmp("ICC", key.pData_, 3) == 0 || - memcmp("Description", key.pData_, 11) == 0) + if (compare("Raw profile type exif", key, 21) || + compare("Raw profile type APP1", key, 21) || + compare("Raw profile type iptc", key, 21) || + compare("Raw profile type xmp", key, 20) || + compare("XML:com.adobe.xmp", key, 17) || + compare("icc", key, 3) || // see test/data/imagemagick.png + compare("ICC", key, 3) || + compare("Description", key, 11)) { #ifdef DEBUG std::cout << "Exiv2::PngImage::doWriteMetadata: strip " << szChunk