From c73d1e27198a389ce7caf52ac30f8e2120acdafd Mon Sep 17 00:00:00 2001 From: Kevin Backhouse Date: Fri, 26 Apr 2019 10:21:46 +0100 Subject: [PATCH 1/1] Avoid negative integer overflow when `filesize < io_->tell()`. This fixes #791. --- include/exiv2/basicio.hpp | 21 +++++++++++++++++++++ include/exiv2/webpimage.hpp | 2 +- src/basicio.cpp | 14 ++++++++++++++ src/webpimage.cpp | 26 +++++++++++++++----------- diff -Naur a/include/exiv2/basicio.hpp b/include/exiv2/basicio.hpp --- a/include/exiv2/basicio.hpp 2020-05-21 17:33:11.452872734 +0800 +++ b/include/exiv2/basicio.hpp 2020-05-21 18:25:25.577619496 +0800 @@ -28,6 +28,7 @@ // ***************************************************************************** // included header files #include "types.hpp" +#include "error.hpp" #include "futils.hpp" // + standard includes @@ -155,6 +156,26 @@ */ virtual long read(byte* buf, long rcount) = 0; /*! + @brief Read data from the IO source. Reading starts at the current + IO position and the position is advanced by the number of bytes + read. Throw an exception if not enough bytes are available. + @param rcount Number of bytes to read. + @param err ErrorCode to throw if not enough bytes are available. + @return DataBuf instance containing the bytes read. + */ + DataBuf readOrThrow(long rcount, ErrorCode err); + /*! + @brief Read data from the IO source. Reading starts at the current + IO position and the position is advanced by the number of bytes + read. Throw an exception if not enough bytes are available. + @param buf Pointer to a block of memory into which the read data + is stored. The memory block must be at least \em rcount bytes + long. + @param err ErrorCode to throw if not enough bytes are available. + @param rcount Number of bytes to read. + */ + void readOrThrow(byte* buf, long rcount, ErrorCode err); + /*! @brief Read one byte from the IO source. Current IO position is advanced by one byte. @return The byte read from the IO source if successful;
diff -Naur a/include/exiv2/webpimage.hpp b/include/exiv2/webpimage.hpp --- a/include/exiv2/webpimage.hpp 2020-05-21 17:33:11.452872734 +0800 +++ b/include/exiv2/webpimage.hpp 2020-05-21 18:27:02.307763543 +0800 @@ -95,7 +95,7 @@ byte *header, long header_size); bool equalsWebPTag(Exiv2::DataBuf& buf ,const char* str); void debugPrintHex(byte *data, long size); - void decodeChunks(uint64_t filesize); + void decodeChunks(uint32_t filesize); void inject_VP8X(BasicIo& iIo, bool has_xmp, bool has_exif, bool has_alpha, bool has_icc, int width, int height); diff -Naur a/src/basicio.cpp b/src/basicio.cpp --- a/src/basicio.cpp 2020-05-21 17:33:11.472872765 +0800 +++ b/src/basicio.cpp 2020-05-21 18:29:18.497966466 +0800 @@ -33,6 +33,7 @@ #include "futils.hpp" #include "types.hpp" #include "error.hpp" +#include "enforce.hpp" #include "http.hpp" #include "properties.hpp" @@ -83,6 +84,18 @@ BasicIo::~BasicIo() { } + DataBuf BasicIo::readOrThrow(long rcount, ErrorCode err) { + DataBuf result = read(rcount); + enforce(result.size_ == rcount, err); + enforce(!error(), err); + return result; + } + + void BasicIo::readOrThrow(byte* buf, long rcount, ErrorCode err) { + const long nread = read(buf, rcount); + enforce(nread == rcount, err); + enforce(!error(), err); + } //! Internal Pimpl structure of class FileIo. class FileIo::Impl { diff -Naur a/src/webpimage.cpp b/src/webpimage.cpp --- a/src/webpimage.cpp 2020-05-21 17:33:11.472872765 +0800 +++ b/src/webpimage.cpp 2020-05-21 19:05:02.951135410 +0800 @@ -493,7 +493,7 @@ DataBuf chunkId(5); chunkId.pData_[4] = '\0' ; - io_->read(data, WEBP_TAG_SIZE * 3); + io_->readOrThrow(data, WEBP_TAG_SIZE * 3, Exiv2::kerCorruptedMetadata); const uint32_t filesize = Exiv2::getULong(data + WEBP_TAG_SIZE, littleEndian) + 8; enforce(filesize <= io_->size(), Exiv2::kerCorruptedMetadata); @@ -501,7 +501,7 @@ } // WebPImage::readMetadata - void WebPImage::decodeChunks(uint64_t filesize) + void WebPImage::decodeChunks(uint32_t filesize) { DataBuf chunkId(5); byte size_buff[WEBP_TAG_SIZE]; @@ -513,9 +513,10 @@ chunkId.pData_[4] = '\0' ; while ( !io_->eof() && (uint64_t) io_->tell() < filesize) { - io_->read(chunkId.pData_, WEBP_TAG_SIZE); - io_->read(size_buff, WEBP_TAG_SIZE); + io_->readOrThrow(chunkId.pData_, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); + io_->readOrThrow(size_buff, WEBP_TAG_SIZE, Exiv2::kerCorruptedMetadata); const uint32_t size = Exiv2::getULong(size_buff, littleEndian); + enforce(io_->tell() <= filesize, Exiv2::kerCorruptedMetadata); enforce(size <= (filesize - io_->tell()), Exiv2::kerCorruptedMetadata); DataBuf payload(size); @@ -526,7 +527,7 @@ has_canvas_data = true; byte size_buf[WEBP_TAG_SIZE]; - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); // Fetch width memcpy(&size_buf, &payload.pData_[4], 3); @@ -541,7 +542,7 @@ enforce(size >= 10, Exiv2::kerCorruptedMetadata); has_canvas_data = true; - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); byte size_buf[WEBP_TAG_SIZE]; // Fetch width"" @@ -562,7 +563,7 @@ byte size_buf_w[2]; byte size_buf_h[3]; - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); // Fetch width memcpy(&size_buf_w, &payload.pData_[1], 2); @@ -580,7 +581,7 @@ has_canvas_data = true; byte size_buf[WEBP_TAG_SIZE]; - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); // Fetch width memcpy(&size_buf, &payload.pData_[6], 3); @@ -592,10 +593,10 @@ size_buf[3] = 0; pixelHeight_ = Exiv2::getULong(size_buf, littleEndian) + 1; } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_ICCP)) { - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); this->setIccProfile(payload); } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_EXIF)) { - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); byte size_buff[2]; byte exifLongHeader[] = { 0xFF, 0x01, 0xFF, 0xE1 }; @@ -676,7 +677,7 @@ if (rawExifData) free(rawExifData); } else if (equalsWebPTag(chunkId, WEBP_CHUNK_HEADER_XMP)) { - io_->read(payload.pData_, payload.size_); + io_->readOrThrow(payload.pData_, payload.size_, Exiv2::kerCorruptedMetadata); xmpPacket_.assign(reinterpret_cast(payload.pData_), payload.size_); if (xmpPacket_.size() > 0 && XmpParser::decode(xmpData_, xmpPacket_)) { #ifndef SUPPRESS_WARNINGS @@ -708,7 +709,10 @@ } bool isWebPType(BasicIo& iIo, bool /*advance*/) - { + { + if (iIo.size() < 12) { + return false; + } const int32_t len = 4; const unsigned char RiffImageId[4] = { 'R', 'I', 'F' ,'F'}; const unsigned char WebPImageId[4] = { 'W', 'E', 'B' ,'P'};