diff --git a/include/exiv2/CMakeLists.txt b/include/exiv2/CMakeLists.txt index 60080f1c..ccd9dfc6 100644 --- a/include/exiv2/CMakeLists.txt +++ b/include/exiv2/CMakeLists.txt @@ -1,7 +1,6 @@ install(FILES asfvideo.hpp basicio.hpp - bigtiffimage.hpp bmpimage.hpp config.h convert.hpp diff --git a/include/exiv2/bigtiffimage.hpp b/include/exiv2/bigtiffimage.hpp deleted file mode 100644 index 9e53ee8a..00000000 --- a/include/exiv2/bigtiffimage.hpp +++ /dev/null @@ -1,16 +0,0 @@ - -#include "basicio.hpp" -#include "image.hpp" - -namespace Exiv2 -{ - -namespace ImageType -{ - const int bigtiff = 25; -} - -Image::AutoPtr newBigTiffInstance(BasicIo::AutoPtr, bool); -bool isBigTiffType(BasicIo &, bool); - -} diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 56be9516..83cbbb00 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -50,7 +50,6 @@ add_library( exiv2lib ../include/exiv2/rwlock.hpp ../include/exiv2/slice.hpp basicio.cpp ../include/exiv2/basicio.hpp - bigtiffimage.cpp bmpimage.cpp ../include/exiv2/bmpimage.hpp convert.cpp ../include/exiv2/convert.hpp cr2image.cpp ../include/exiv2/cr2image.hpp diff --git a/src/bigtiffimage.cpp b/src/bigtiffimage.cpp deleted file mode 100644 index 68a57f90..00000000 --- a/src/bigtiffimage.cpp +++ /dev/null @@ -1,472 +0,0 @@ -#include "bigtiffimage.hpp" - -#include "safe_op.hpp" -#include "exif.hpp" -#include "error.hpp" -#include "image_int.hpp" -#include "enforce.hpp" - -#include -#include -#include - -namespace Exiv2 -{ - - namespace - { - struct Header - { - enum Format - { - StandardTiff, - BigTiff, - }; - - Header(): byteOrder_(invalidByteOrder), version_(-1), data_size_(0), dir_offset_(0) {} - Header(const ByteOrder& order, int v, int size, uint64_t offset): - byteOrder_(order), - version_(v), - data_size_(size), - dir_offset_(offset) - { - - } - - bool isValid() const - { - return version_ != -1; - } - - ByteOrder byteOrder() const - { - assert(isValid()); - return byteOrder_; - } - - int version() const - { - assert(isValid()); - return version_; - } - - Format format() const - { - assert(isValid()); - return version_ == 0x2A? StandardTiff: BigTiff; - } - - int dataSize() const - { - assert(isValid()); - return data_size_; - } - - uint64_t dirOffset() const - { - assert(isValid()); - return dir_offset_; - } - - private: - ByteOrder byteOrder_; - int version_; // 42 or 43 - regular tiff or big tiff - int data_size_; // 4 or 8 - uint64_t dir_offset_; - }; - - Header readHeader(BasicIo& io) - { - byte header[2] = {0, 0}; - io.read(header, 2); - - ByteOrder byteOrder = invalidByteOrder; - if (header[0] == 'I' && header[1] == 'I') - byteOrder = littleEndian; - else if (header[0] == 'M' && header[1] == 'M') - byteOrder = bigEndian; - - if (byteOrder == invalidByteOrder) - return Header(); - - byte version[2] = {0, 0}; - io.read(version, 2); - - const uint16_t magic = getUShort(version, byteOrder); - - if (magic != 0x2A && magic != 0x2B) - return Header(); - - Header result; - - if (magic == 0x2A) - { - byte buffer[4]; - int read = io.read(buffer, 4); - - if (read < 4) - throw Exiv2::Error(kerCorruptedMetadata); - - const uint32_t offset = getULong(buffer, byteOrder); - result = Header(byteOrder, magic, 4, offset); - } - else - { - byte buffer[8] = {0, 0, 0, 0, 0, 0, 0, 0}; - int read = io.read(buffer, 2); - if (read < 2) - throw Exiv2::Error(kerCorruptedMetadata); - - const int size = getUShort(buffer, byteOrder); - - if (size == 8) - { - read = io.read(buffer, 2); // null - if (read < 2) - throw Exiv2::Error(kerCorruptedMetadata); - - read = io.read(buffer, 8); - if (read < 8) - throw Exiv2::Error(kerCorruptedMetadata); - - const uint64_t offset = getULongLong(buffer, byteOrder); - - if (offset >= io.size()) - throw Exiv2::Error(kerCorruptedMetadata); - - result = Header(byteOrder, magic, size, offset); - } - else - throw Exiv2::Error(kerCorruptedMetadata); - } - - return result; - } - - class BigTiffImage: public Image - { - public: - BigTiffImage(BasicIo::AutoPtr io): - Image(ImageType::bigtiff, mdExif, io), - header_(), - dataSize_(0), - doSwap_(false) - { - header_ = readHeader(Image::io()); - assert(header_.isValid()); - - doSwap_ = (isLittleEndianPlatform() && header_.byteOrder() == bigEndian) - || (isBigEndianPlatform() && header_.byteOrder() == littleEndian); - - dataSize_ = header_.format() == Header::StandardTiff? 4 : 8; - } - - virtual ~BigTiffImage() {} - - // overrides - void readMetadata() - { - - } - - void writeMetadata() - { - - } - - std::string mimeType() const - { - return std::string(); - } - - void printStructure(std::ostream& os, PrintStructureOption option, int depth) - { - printIFD(os, option, header_.dirOffset(), depth - 1); - } - - private: - Header header_; - int dataSize_; - bool doSwap_; - - void printIFD(std::ostream& out, PrintStructureOption option, uint64_t dir_offset, int depth) - { - BasicIo& io = Image::io(); - - // Fix for https://github.com/Exiv2/exiv2/issues/712 - // A malicious file can cause a very deep recursion, leading to - // stack exhaustion. - // Note: 200 is an arbitrarily chosen cut-off value. The value - // of depth determines the amount of indentation inserted by the - // pretty-printer. The output starts to become unreadable as - // soon as the indentation exceeds 80 characters or so. That's - // why 200 ought to be a reasonable cut-off. - if (depth > 200) { - out << Internal::indent(depth) << "Maximum indentation depth exceeded." << std::endl; - return; - } - - depth++; - bool bFirst = true; - - // buffer - bool bPrint = true; - - do - { - // Read top of directory - io.seek(dir_offset, BasicIo::beg); - - const uint64_t entries = readData(header_.format() == Header::StandardTiff? 2: 8); - const bool tooBig = entries > 500; - - if ( bFirst && bPrint ) - { - out << Internal::indent(depth) << "STRUCTURE OF BIGTIFF FILE " << io.path() << std::endl; - if (tooBig) - out << Internal::indent(depth) << "entries = " << entries << std::endl; - } - - if (tooBig) - break; - - // Read the dictionary - for ( uint64_t i = 0; i < entries; i ++ ) - { - if ( bFirst && bPrint ) - out << Internal::indent(depth) - << " address | tag | " - << " type | count | offset | value\n"; - - bFirst = false; - - const uint16_t tag = (uint16_t) readData(2); - const uint16_t type = (uint16_t) readData(2); - const uint64_t count = readData(dataSize_); - const DataBuf data = io.read(dataSize_); // Read data as raw value. what should be done about it will be decided depending on type - - std::string sp = "" ; // output spacer - - //prepare to print the value - // TODO: figure out what's going on with kount - const uint64_t kount = isStringType(type)? (count > 32 ? 32 : count) // restrict long arrays - : count > 5 ? 5 - : count - ; - const uint32_t pad = isStringType(type) ? 1 : 0; - const uint32_t size = isStringType(type) ? 1 - : is2ByteType(type) ? 2 - : is4ByteType(type) ? 4 - : is8ByteType(type) ? 8 - : 1; - - // #55 and #56 memory allocation crash test/data/POC8 - - // size * count > std::numeric_limits::max() - // => - // size > std::numeric_limits::max() / count - // (don't perform that check when count == 0 => will cause a division by zero exception) - if (count != 0) { - if (size > std::numeric_limits::max() / count) { - throw Error(kerInvalidMalloc); // we got number bigger than 2^64 - } - } - // more than we can handle - - if (size * count > std::numeric_limits::max() - pad) - throw Error(kerInvalidMalloc); // again more than 2^64 - - const uint64_t allocate = size*count + pad; - if ( allocate > io.size() ) { - throw Error(kerInvalidMalloc); - } - - DataBuf buf(static_cast(allocate)); - - const uint64_t offset = header_.format() == Header::StandardTiff? - byteSwap4(data, 0, doSwap_): - byteSwap8(data, 0, doSwap_); - - // big data? Use 'data' as pointer to real data - const bool usePointer = (size_t) count*size > (size_t) dataSize_; - - if ( usePointer ) // read into buffer - { - size_t restore = io.tell(); // save - io.seek(offset, BasicIo::beg); // position - io.read(buf.pData_, (long) count * size); // read - io.seek(restore, BasicIo::beg); // restore - } - else // use 'data' as data :) - std::memcpy(buf.pData_, data.pData_, (size_t) count * size); // copy data - - if ( bPrint ) - { - const uint64_t entrySize = header_.format() == Header::StandardTiff? 12: 20; - const uint64_t address = dir_offset + 2 + i * entrySize; - - out << Internal::indent(depth) - << Internal::stringFormat("%8u | %#06x %-25s |%10s |%9u |", - static_cast(address), tag, tagName(tag).c_str(), typeName(type), count) - <<(usePointer ? Internal::stringFormat("%10u | ",(size_t)offset) - : Internal::stringFormat("%10s | ","")) - ; - if ( isShortType(type) ) - { - for ( size_t k = 0 ; k < kount ; k++ ) - { - out << sp << byteSwap2(buf, k*size, doSwap_); - sp = " "; - } - } - else if ( isLongType(type) ) - { - for ( size_t k = 0 ; k < kount ; k++ ) - { - out << sp << byteSwap4(buf, k*size, doSwap_); - sp = " "; - } - } - else if ( isLongLongType(type) ) - { - for ( size_t k = 0 ; k < kount ; k++ ) - { - out << sp << byteSwap8(buf, k*size, doSwap_); - sp = " "; - } - } - else if ( isRationalType(type) ) - { - for ( size_t k = 0 ; k < kount ; k++ ) - { - uint32_t a = byteSwap4(buf, k*size+0, doSwap_); - uint32_t b = byteSwap4(buf, k*size+4, doSwap_); - out << sp << a << "/" << b; - sp = " "; - } - } - else if ( isStringType(type) ) - out << sp << Internal::binaryToString(makeSlice(buf, 0, static_cast(kount))); - - sp = kount == count ? "" : " ..."; - out << sp << std::endl; - - if ( option == kpsRecursive && - (tag == 0x8769 /* ExifTag */ || tag == 0x014a/*SubIFDs*/ || type == tiffIfd || type == tiffIfd8) ) - { - for ( size_t k = 0 ; k < count ; k++ ) - { - const size_t restore = io.tell(); - const uint64_t ifdOffset = type == tiffIfd8? - byteSwap8(buf, k*size, doSwap_): - byteSwap4(buf, k*size, doSwap_); - - printIFD(out, option, ifdOffset, depth); - io.seek(restore, BasicIo::beg); - } - } - else if ( option == kpsRecursive && tag == 0x83bb /* IPTCNAA */ ) - { - if (Safe::add(count, offset) > io.size()) { - throw Error(kerCorruptedMetadata); - } - - const size_t restore = io.tell(); - io.seek(offset, BasicIo::beg); // position - std::vector bytes((size_t)count) ; // allocate memory - // TODO: once we have C++11 use bytes.data() - const long read_bytes = io.read(&bytes[0], static_cast(count)); - io.seek(restore, BasicIo::beg); - // TODO: once we have C++11 use bytes.data() - IptcData::printStructure(out, makeSliceUntil(&bytes[0], read_bytes), depth); - - } - else if ( option == kpsRecursive && tag == 0x927c /* MakerNote */ && count > 10) - { - size_t restore = io.tell(); // save - - long jump= 10 ; - byte bytes[20] ; - const char* chars = (const char*) &bytes[0] ; - io.seek(dir_offset, BasicIo::beg); // position - io.read(bytes,jump ) ; // read - bytes[jump]=0 ; - if ( ::strcmp("Nikon",chars) == 0 ) - { - // tag is an embedded tiff - std::vector nikon_bytes((size_t)(count - jump)); - - io.read(&nikon_bytes.at(0), (long)nikon_bytes.size()); - MemIo memIo(&nikon_bytes.at(0), (long)count - jump); // create a file - std::cerr << "Nikon makernote" << std::endl; - // printTiffStructure(memIo,out,option,depth); - // TODO: fix it - } - else - { - // tag is an IFD - io.seek(0, BasicIo::beg); // position - std::cerr << "makernote" << std::endl; - printIFD(out,option,offset,depth); - } - - io.seek(restore,BasicIo::beg); // restore - } - } - } - - const uint64_t nextDirOffset = readData(dataSize_); - - dir_offset = tooBig ? 0 : nextDirOffset; - out.flush(); - } while (dir_offset != 0); - - if ( bPrint ) - out << Internal::indent(depth) << "END " << io.path() << std::endl; - - depth--; - } - - uint64_t readData(int size) const - { - const DataBuf data = Image::io().read(size); - enforce(data.size_ != 0, kerCorruptedMetadata); - - uint64_t result = 0; - - if (data.size_ == 1) - {} - else if (data.size_ == 2) - result = byteSwap2(data, 0, doSwap_); - else if (data.size_ == 4) - result = byteSwap4(data, 0, doSwap_); - else if (data.size_ == 8) - result = byteSwap8(data, 0, doSwap_); - else - throw Exiv2::Error(kerCorruptedMetadata); - - return result; - } - }; - } - - - Image::AutoPtr newBigTiffInstance(BasicIo::AutoPtr io, bool) - { - return Image::AutoPtr(new BigTiffImage(io)); - } - - - bool isBigTiffType(BasicIo& io, bool advance) - { - const long pos = io.tell(); - const Header header = readHeader(io); - const bool valid = header.isValid(); - - if (valid == false || advance == false) - io.seek(pos, BasicIo::beg); - - return valid; - } - -} diff --git a/src/image.cpp b/src/image.cpp index 64451703..c0d0633a 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -44,7 +44,6 @@ #include "tiffimage_int.hpp" #include "tiffcomposite_int.hpp" #include "tiffvisitor_int.hpp" -#include "bigtiffimage.hpp" #include "webpimage.hpp" #include "orfimage.hpp" #include "gifimage.hpp" @@ -111,7 +110,6 @@ namespace { { ImageType::crw, newCrwInstance, isCrwType, amReadWrite, amNone, amNone, amReadWrite }, { ImageType::mrw, newMrwInstance, isMrwType, amRead, amRead, amRead, amNone }, { ImageType::tiff, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone }, - { ImageType::bigtiff, newBigTiffInstance, isBigTiffType, amRead, amRead, amRead, amNone }, { ImageType::webp, newWebPInstance, isWebPType, amReadWrite, amNone, amReadWrite, amNone }, { ImageType::dng, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone }, { ImageType::nef, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone }, diff --git a/src/tgaimage.cpp b/src/tgaimage.cpp index 3ddb9713..5e61a363 100644 --- a/src/tgaimage.cpp +++ b/src/tgaimage.cpp @@ -153,6 +153,8 @@ namespace Exiv2 { #endif byte buf[26]; long curPos = iIo.tell(); + if ( curPos < 26 ) return false; + iIo.seek(-26, BasicIo::end); if (iIo.error() || iIo.eof()) { diff --git a/tests/bugfixes/github/test_CVE_2017_17722.py b/tests/bugfixes/github/test_CVE_2017_17722.py index 65f2d825..d059b63a 100644 --- a/tests/bugfixes/github/test_CVE_2017_17722.py +++ b/tests/bugfixes/github/test_CVE_2017_17722.py @@ -13,5 +13,5 @@ class TestCvePoC(metaclass=system_tests.CaseMeta): stdout = [""] stderr = [ """$exiv2_exception_message """ + filename + """: -$kerCorruptedMetadata +$filename: $kerFileContainsUnknownImageType """] diff --git a/tests/bugfixes/github/test_issue_262.py b/tests/bugfixes/github/test_issue_262.py index eadfd91c..7879cb9f 100644 --- a/tests/bugfixes/github/test_issue_262.py +++ b/tests/bugfixes/github/test_issue_262.py @@ -11,15 +11,10 @@ class DivByZeroInPrintIFD(metaclass=system_tests.CaseMeta): "$data_path/7-printIFD-divbyzero-1" ) commands = ["$exiv2 -pX $filename"] - stdout = [ - """STRUCTURE OF BIGTIFF FILE $filename - address | tag | type | count | offset | value - 10 | 0x0008 FlashSetting | unknown | 0 | | -""" - ] + stdout = [""] stderr = [ """$exiv2_exception_message $filename: -$kerCorruptedMetadata +$filename: $kerFileContainsUnknownImageType """ ] retval = [1] diff --git a/tests/bugfixes/github/test_issue_712.py b/tests/bugfixes/github/test_issue_712.py index 0005b916..3d3ca508 100644 --- a/tests/bugfixes/github/test_issue_712.py +++ b/tests/bugfixes/github/test_issue_712.py @@ -4,6 +4,10 @@ import system_tests class BigTiffImageRecursionStackExhaustion( metaclass=system_tests.CaseMeta): """ + src/bigtiffimage.cpp is longer in the code base + however, let's retain this test as support for BigTiff will + be developed and the malicious test file may stress the bigtiff parser. + Regression test for the bug described in: https://github.com/Exiv2/exiv2/issues/712 @@ -19,12 +23,9 @@ class BigTiffImageRecursionStackExhaustion( "$data_path/issue_712_poc.tif" ) commands = ["$exiv2 -b -u -k pr $filename"] - stdout = ["File name : " + filename + """ -File size : 3720 Bytes -MIME type : -Image size : 0 x 0 -""" -] - stderr = [filename + """: No Exif data found in the file + stdout = [""] + stderr = [ + """$exiv2_exception_message """ + filename + """: +$filename: $kerFileContainsUnknownImageType """] - retval = [253] + retval = [1] diff --git a/tests/suite.conf b/tests/suite.conf index 1cda8568..719d5096 100644 --- a/tests/suite.conf +++ b/tests/suite.conf @@ -32,6 +32,7 @@ kerInvalidMalloc: invalid memory allocation request kerInvalidTypeValue: invalid type in tiff structure kerNotAJpeg : This does not look like a JPEG image kerNoImageInInputData: Input data does not contain a valid image +kerFileContainsUnknownImageType: The file contains data of an unknown image type addition_overflow_message: Overflow in addition exiv2_exception_message: Exiv2 exception in print action for file exiv2_overflow_exception_message: std::overflow_error exception in print action for file