diff --git a/src/image.cpp b/src/image.cpp index 0fdc4e41b8..1428e6b009 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -24,6 +24,7 @@ #include "image.hpp" #include "image_int.hpp" #include "error.hpp" +#include "enforce.hpp" #include "futils.hpp" #include "safe_op.hpp" #include "slice.hpp" @@ -137,6 +138,19 @@ namespace { // ***************************************************************************** // class member definitions namespace Exiv2 { + // BasicIo::read() with error checking + static void readOrThrow(BasicIo& iIo, byte* buf, long rcount, ErrorCode err) { + const long nread = iIo.read(buf, rcount); + enforce(nread == rcount, err); + enforce(!iIo.error(), err); + } + + // BasicIo::seek() with error checking + static void seekOrThrow(BasicIo& iIo, long offset, BasicIo::Position pos, ErrorCode err) { + const int r = iIo.seek(offset, pos); + enforce(r == 0, err); + } + Image::Image(int imageType, uint16_t supportedMetadata, BasicIo::UniquePtr io) : io_(std::move(io)), pixelWidth_(0), @@ -328,11 +342,8 @@ namespace Exiv2 { do { // Read top of directory - const int seekSuccess = !io.seek(start,BasicIo::beg); - const long bytesRead = io.read(dir.pData_, 2); - if (!seekSuccess || bytesRead == 0) { - throw Error(kerCorruptedMetadata); - } + seekOrThrow(io, start, BasicIo::beg, kerCorruptedMetadata); + readOrThrow(io, dir.pData_, 2, kerCorruptedMetadata); uint16_t dirLength = byteSwap2(dir,0,bSwap); // Prevent infinite loops. (GHSA-m479-7frc-gqqg) enforce(dirLength > 0, kerCorruptedMetadata); @@ -358,7 +369,7 @@ namespace Exiv2 { } bFirst = false; - io.read(dir.pData_, 12); + readOrThrow(io, dir.pData_, 12, kerCorruptedMetadata); uint16_t tag = byteSwap2(dir,0,bSwap); uint16_t type = byteSwap2(dir,2,bSwap); uint32_t count = byteSwap4(dir,4,bSwap); @@ -391,20 +402,27 @@ namespace Exiv2 { // if ( offset > io.size() ) offset = 0; // Denial of service? // #55 and #56 memory allocation crash test/data/POC8 - long long allocate = static_cast(size) * count + pad + 20; - if (allocate > static_cast(io.size())) { + const uint64_t allocate64 = static_cast(size) * count + pad + 20; + if ( allocate64 > io.size() ) { throw Error(kerInvalidMalloc); } - DataBuf buf(static_cast(allocate)); // allocate a buffer + // Overflow check + enforce(allocate64 <= static_cast(std::numeric_limits::max()), kerCorruptedMetadata); + enforce(allocate64 <= static_cast(std::numeric_limits::max()), kerCorruptedMetadata); + const long allocate = static_cast(allocate64); + DataBuf buf(allocate); // allocate a buffer std::memset(buf.pData_, 0, buf.size_); std::memcpy(buf.pData_,dir.pData_+8,4); // copy dir[8:11] into buffer (short strings) - const bool bOffsetIsPointer = count*size > 4; + + // We have already checked that this multiplication cannot overflow. + const uint32_t count_x_size = count*size; + const bool bOffsetIsPointer = count_x_size > 4; if ( bOffsetIsPointer ) { // read into buffer - size_t restore = io.tell(); // save - io.seek(offset,BasicIo::beg); // position - io.read(buf.pData_,count*size);// read - io.seek(restore,BasicIo::beg); // restore + const long restore = io.tell(); // save + seekOrThrow(io, offset, BasicIo::beg, kerCorruptedMetadata); // position + readOrThrow(io, buf.pData_, static_cast(count_x_size), kerCorruptedMetadata); // read + seekOrThrow(io, restore, BasicIo::beg, kerCorruptedMetadata); // restore } if ( bPrint ) { @@ -443,10 +461,10 @@ namespace Exiv2 { if ( option == kpsRecursive && (tag == 0x8769 /* ExifTag */ || tag == 0x014a/*SubIFDs*/ || type == tiffIfd) ) { for ( size_t k = 0 ; k < count ; k++ ) { - size_t restore = io.tell(); + const long restore = io.tell(); offset = byteSwap4(buf,k*size,bSwap); printIFDStructure(io,out,option,offset,bSwap,c,depth); - io.seek(restore,BasicIo::beg); + seekOrThrow(io, restore, BasicIo::beg, kerCorruptedMetadata); } } else if ( option == kpsRecursive && tag == 0x83bb /* IPTCNAA */ ) { @@ -454,38 +472,38 @@ namespace Exiv2 { throw Error(kerCorruptedMetadata); } - const size_t restore = io.tell(); - io.seek(offset, BasicIo::beg); // position + const long restore = io.tell(); + seekOrThrow(io, offset, BasicIo::beg, kerCorruptedMetadata); // position std::vector bytes(count) ; // allocate memory // TODO: once we have C++11 use bytes.data() - const long read_bytes = io.read(&bytes[0], count); - io.seek(restore, BasicIo::beg); + readOrThrow(io, &bytes[0], count, kerCorruptedMetadata); + seekOrThrow(io, restore, BasicIo::beg, kerCorruptedMetadata); // TODO: once we have C++11 use bytes.data() - IptcData::printStructure(out, makeSliceUntil(&bytes[0], read_bytes), depth); + IptcData::printStructure(out, makeSliceUntil(&bytes[0], count), depth); } else if ( option == kpsRecursive && tag == 0x927c /* MakerNote */ && count > 10) { - size_t restore = io.tell(); // save + const long restore = io.tell(); // save uint32_t jump= 10 ; byte bytes[20] ; const auto chars = reinterpret_cast(&bytes[0]); - io.seek(offset,BasicIo::beg); // position - io.read(bytes,jump ) ; // read + seekOrThrow(io, offset, BasicIo::beg, kerCorruptedMetadata); // position + readOrThrow(io, bytes, jump, kerCorruptedMetadata) ; // read bytes[jump]=0 ; if ( ::strcmp("Nikon",chars) == 0 ) { // tag is an embedded tiff - auto bytes2 = new byte[count - jump]; // allocate memory - io.read(bytes2,count-jump) ; // read - MemIo memIo(bytes2,count-jump) ; // create a file + const long byteslen = count-jump; + DataBuf bytes(byteslen); // allocate a buffer + readOrThrow(io, bytes.pData_, byteslen, kerCorruptedMetadata); // read + MemIo memIo(bytes.pData_, byteslen) ; // create a file printTiffStructure(memIo,out,option,depth); - delete[] bytes2 ; // free } else { // tag is an IFD - io.seek(0,BasicIo::beg); // position + seekOrThrow(io, 0, BasicIo::beg, kerCorruptedMetadata); // position printIFDStructure(io,out,option,offset,bSwap,c,depth); } - io.seek(restore,BasicIo::beg); // restore + seekOrThrow(io, restore, BasicIo::beg, kerCorruptedMetadata); // restore } } @@ -498,7 +516,7 @@ namespace Exiv2 { } } if ( start ) { - io.read(dir.pData_, 4); + readOrThrow(io, dir.pData_, 4, kerCorruptedMetadata); start = byteSwap4(dir,0,bSwap); } } while (start) ; @@ -518,7 +536,7 @@ namespace Exiv2 { DataBuf dir(dirSize); // read header (we already know for certain that we have a Tiff file) - io.read(dir.pData_, 8); + readOrThrow(io, dir.pData_, 8, kerCorruptedMetadata); char c = static_cast(dir.pData_[0]); bool bSwap = ( c == 'M' && isLittleEndianPlatform() ) || ( c == 'I' && isBigEndianPlatform() )