diff --git a/ci/run.sh b/ci/run.sh index 941e05b4..12e3094b 100755 --- a/ci/run.sh +++ b/ci/run.sh @@ -15,7 +15,11 @@ fi mkdir build && cd build conan install .. --build missing --profile release -cmake ${CMAKE_OPTIONS} -DCMAKE_INSTALL_PREFIX=install .. +cmake ${CMAKE_OPTIONS} -DCMAKE_INSTALL_PREFIX=install \ + -DCMAKE_CXX_FLAGS="-fsanitize=address" \ + -DCMAKE_C_FLAGS="-fsanitize=address" \ + -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" \ + -DCMAKE_MODULE_LINKER_FLAGS="-fsanitize=address" .. make -j2 VERBOSE=1 make tests make install diff --git a/include/exiv2/types.hpp b/include/exiv2/types.hpp index 1e8d390c..21f58a0e 100644 --- a/include/exiv2/types.hpp +++ b/include/exiv2/types.hpp @@ -85,6 +85,14 @@ //! Simple common max macro #define EXV_MAX(a,b) ((a) > (b) ? (a) : (b)) +#if defined(__GNUC__) && (__GNUC__ >= 4) || defined(__clang__) +#define EXV_WARN_UNUSED_RESULT __attribute__ ((warn_unused_result)) +#elif defined(_MSC_VER) && (_MSC_VER >= 1700) +#define EXV_WARN_UNUSED_RESULT _Check_return_ +#else +#define EXV_WARN_UNUSED_RESULT +#endif + // ***************************************************************************** // forward declarations struct tm; @@ -235,7 +243,13 @@ namespace Exiv2 { buffer as a data pointer and size pair, resets the internal buffer. */ - std::pair release(); + EXV_WARN_UNUSED_RESULT std::pair release(); + + /*! + @brief Free the internal buffer and reset the size to 0. + */ + void free(); + //! Reset value void reset(std::pair =std::make_pair((byte*)(0),long(0))); //@} diff --git a/src/basicio.cpp b/src/basicio.cpp index 87751915..33818ca7 100644 --- a/src/basicio.cpp +++ b/src/basicio.cpp @@ -2460,11 +2460,12 @@ namespace Exiv2 { if (protocol_ == pSftp) { if (sftp_seek(fileHandler_, (uint32_t) (lowBlock * blockSize_)) < 0) throw Error(kerErrorMessage, "SFTP: unable to sftp_seek"); size_t buffSize = (highBlock - lowBlock + 1) * blockSize_; - char* buffer = new char[buffSize]; - long nBytes = (long) sftp_read(fileHandler_, buffer, buffSize); - if (nBytes < 0) throw Error(kerErrorMessage, "SFTP: unable to sftp_read"); - response.assign(buffer, buffSize); - delete[] buffer; + std::vector buffer(buffSize); + long nBytes = static_cast(sftp_read(fileHandler_, &buffer.at(0), buffSize)); + if (nBytes < 0) { + throw Error(kerErrorMessage, "SFTP: unable to sftp_read"); + } + response.assign(&buffer.at(0), buffSize); } else { std::stringstream ss; if (lowBlock > -1 && highBlock > -1) { diff --git a/src/image.cpp b/src/image.cpp index bd6ef531..5277c58e 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -662,7 +662,7 @@ namespace Exiv2 { void Image::clearIccProfile() { - iccProfile_.release(); + iccProfile_.free(); } void Image::setByteOrder(ByteOrder byteOrder) diff --git a/src/iptc.cpp b/src/iptc.cpp index ec7a58a9..8436b6db 100644 --- a/src/iptc.cpp +++ b/src/iptc.cpp @@ -348,24 +348,30 @@ namespace Exiv2 { return iptcMetadata_.erase(pos); } - void IptcData::printStructure(std::ostream& out, const byte* bytes,const size_t size,uint32_t depth) - { - uint32_t i = 0 ; - while ( i < size-3 && bytes[i] != 0x1c ) i++; - depth++; - out << Internal::indent(depth) << "Record | DataSet | Name | Length | Data" << std::endl; - while ( bytes[i] == 0x1c && i < size-3 ) { - char buff[100]; - uint16_t record = bytes[i+1]; - uint16_t dataset = bytes[i+2]; - uint16_t len = getUShort(bytes+i+3,bigEndian); - sprintf(buff," %6d | %7d | %-24s | %6d | ",record,dataset, Exiv2::IptcDataSets::dataSetName(dataset,record).c_str(), len); + void IptcData::printStructure(std::ostream& out, const byte* bytes, const size_t size, uint32_t depth) + { + uint32_t i = 0; + while (i < size - 3 && bytes[i] != 0x1c) + i++; + depth++; + out << Internal::indent(depth) << "Record | DataSet | Name | Length | Data" << std::endl; + while (i < size - 3) { + if (bytes[i] != 0x1c) { + break; + } + char buff[100]; + uint16_t record = bytes[i + 1]; + uint16_t dataset = bytes[i + 2]; + uint16_t len = getUShort(bytes + i + 3, bigEndian); + sprintf(buff, " %6d | %7d | %-24s | %6d | ", record, dataset, + Exiv2::IptcDataSets::dataSetName(dataset, record).c_str(), len); - out << buff << Internal::binaryToString(bytes,(len>40?40:len),i+5) << (len>40?"...":"") << std::endl; - i += 5 + len; - } - depth--; - } + out << buff << Internal::binaryToString(bytes, (len > 40 ? 40 : len), i + 5) << (len > 40 ? "..." : "") + << std::endl; + i += 5 + len; + } + depth--; + } const char *IptcData::detectCharset() const { diff --git a/src/pngimage.cpp b/src/pngimage.cpp index 17de5bf8..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 { @@ -99,13 +109,14 @@ namespace Exiv2 { zlibResult = uncompress((Bytef*)result.pData_,&uncompressedLen,bytes,length); // if result buffer is large than necessary, redo to fit perfectly. if (zlibResult == Z_OK && (long) uncompressedLen < result.size_ ) { - result.release(); + result.free(); + result.alloc(uncompressedLen); zlibResult = uncompress((Bytef*)result.pData_,&uncompressedLen,bytes,length); } if (zlibResult == Z_BUF_ERROR) { // the uncompressed buffer needs to be larger - result.release(); + result.free(); // Sanity - never bigger than 16mb if (uncompressedLen > 16*1024*1024) zlibResult = Z_DATA_ERROR; @@ -126,10 +137,10 @@ namespace Exiv2 { zlibResult = compress((Bytef*)result.pData_,&compressedLen,bytes,length); if (zlibResult == Z_BUF_ERROR) { // the compressedArray needs to be larger - result.release(); + result.free(); compressedLen *= 2; } else { - result.release(); + result.free(); result.alloc(compressedLen); zlibResult = compress((Bytef*)result.pData_,&compressedLen,bytes,length); } @@ -154,12 +165,21 @@ namespace Exiv2 { } // calculate length and allocate result; + // count: number of \n in the header long count=0; + // p points to the current position in the array bytes const byte* p = bytes ; - // header is \nsomething\n number\n hex - while ( count < 3 ) - if ( *p++ == '\n' ) + + // header is '\nsomething\n number\n hex' + // => increment p until it points to the byte after the last \n + // p must stay within bounds of the bytes array! + while ((count < 3) && (p - bytes < length)) { + // length is later used for range checks of p => decrement it for each increment of p + --length; + if ( *p++ == '\n' ) { count++; + } + } for ( long i = 0 ; i < length ; i++ ) if ( value[p[i]] ) ++count; @@ -678,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 diff --git a/src/types.cpp b/src/types.cpp index 33ae8879..867cfc03 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -155,7 +155,7 @@ namespace Exiv2 { } } - std::pair DataBuf::release() + EXV_WARN_UNUSED_RESULT std::pair DataBuf::release() { std::pair p = std::make_pair(pData_, size_); pData_ = 0; @@ -163,6 +163,13 @@ namespace Exiv2 { return p; } + void DataBuf::free() + { + delete[] pData_; + pData_ = 0; + size_ = 0; + } + void DataBuf::reset(std::pair p) { if (pData_ != p.first) { diff --git a/src/webpimage.cpp b/src/webpimage.cpp index ebb7599d..ae165c46 100644 --- a/src/webpimage.cpp +++ b/src/webpimage.cpp @@ -594,7 +594,8 @@ namespace Exiv2 { io_->read(payload.pData_, payload.size_); byte size_buff[2]; - byte exifLongHeader[] = { 0xFF, 0x01, 0xFF, 0xE1 }; + // 4 meaningful bytes + 2 padding bytes + byte exifLongHeader[] = { 0xFF, 0x01, 0xFF, 0xE1, 0x00, 0x00 }; byte exifShortHeader[] = { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; byte exifTiffLEHeader[] = { 0x49, 0x49, 0x2A }; // "MM*" byte exifTiffBEHeader[] = { 0x4D, 0x4D, 0x00, 0x2A }; // "II\0*"