diff --git a/src/actions.cpp b/src/actions.cpp index 1c854bd2..0fc36290 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -603,8 +603,8 @@ namespace Action { std::ostringstream os; // #1114 - show negative values for SByte if (md.typeId() == Exiv2::signedByte) { - for ( int c = 0 ; c < md.value().count() ; c++ ) { - int value = md.value().toLong(c); + for ( long c = 0 ; c < md.value().count() ; c++ ) { + long value = md.value().toLong(c); os << (c?" ":"") << std::dec << (value < 128 ? value : value - 256); } } else { diff --git a/src/basicio.cpp b/src/basicio.cpp index 6f5b023f..553dbde6 100644 --- a/src/basicio.cpp +++ b/src/basicio.cpp @@ -1780,9 +1780,10 @@ namespace Exiv2 { // find $right findDiff = false; - blockIndex = nBlocks - 1; - blockSize = p_->blocksMap_[blockIndex].getSize(); - while ((blockIndex + 1 > 0) && right < src.size() && !findDiff) { + blockIndex = nBlocks; + while (blockIndex > 0 && right < src.size() && !findDiff) { + blockIndex--; + blockSize = p_->blocksMap_[blockIndex].getSize(); if(src.seek(-1 * (blockSize + right), BasicIo::end)) { findDiff = true; } else { @@ -1797,8 +1798,6 @@ namespace Exiv2 { } } } - blockIndex--; - blockSize = static_cast(p_->blocksMap_[blockIndex].getSize()); } delete []buf; diff --git a/src/convert.cpp b/src/convert.cpp index b131a127..fc603395 100644 --- a/src/convert.cpp +++ b/src/convert.cpp @@ -549,7 +549,7 @@ namespace Exiv2 { auto pos = exifData_->findKey(ExifKey(from)); if (pos == exifData_->end()) return; if (!prepareXmpTarget(to)) return; - for (int i = 0; i < pos->count(); ++i) { + for (long i = 0; i < pos->count(); ++i) { std::string value = pos->toString(i); if (!pos->value().ok()) { #ifndef SUPPRESS_WARNINGS @@ -697,7 +697,7 @@ namespace Exiv2 { if (pos == exifData_->end()) return; if (!prepareXmpTarget(to)) return; std::ostringstream value; - for (int i = 0; i < pos->count(); ++i) { + for (long i = 0; i < pos->count(); ++i) { value << static_cast(pos->toLong(i)); } (*xmpData_)[to] = value.str(); @@ -710,7 +710,7 @@ namespace Exiv2 { if (pos == exifData_->end()) return; if (!prepareXmpTarget(to)) return; std::ostringstream value; - for (int i = 0; i < pos->count(); ++i) { + for (long i = 0; i < pos->count(); ++i) { if (i > 0) value << '.'; value << pos->toLong(i); } @@ -828,7 +828,7 @@ namespace Exiv2 { auto pos = xmpData_->findKey(XmpKey(from)); if (pos == xmpData_->end()) return; std::ostringstream array; - for (int i = 0; i < pos->count(); ++i) { + for (long i = 0; i < pos->count(); ++i) { std::string value = pos->toString(i); if (!pos->value().ok()) { #ifndef SUPPRESS_WARNINGS diff --git a/src/crwimage_int.cpp b/src/crwimage_int.cpp index eac64ddd..a2a28604 100644 --- a/src/crwimage_int.cpp +++ b/src/crwimage_int.cpp @@ -867,12 +867,16 @@ namespace Exiv2 { assert(ifdId != ifdIdNotSet); std::string groupName(Internal::groupName(ifdId)); + const uint32_t component_size = ciffComponent.size(); + enforce(component_size % 2 == 0, kerCorruptedMetadata); + enforce(component_size/2 <= static_cast(std::numeric_limits::max()), kerCorruptedMetadata); + const uint16_t num_components = static_cast(component_size/2); uint16_t c = 1; - while (uint32_t(c)*2 < ciffComponent.size()) { + while (c < num_components) { uint16_t n = 1; ExifKey key(c, groupName); UShortValue value; - if (ifdId == canonCsId && c == 23 && ciffComponent.size() > 50) n = 3; + if (ifdId == canonCsId && c == 23 && component_size >= 52) n = 3; value.read(ciffComponent.pData() + c*2, n*2, byteOrder); image.exifData().add(key, &value); if (ifdId == canonSiId && c == 21) aperture = value.toLong(); diff --git a/src/exif.cpp b/src/exif.cpp index 04f39020..cb2153e9 100644 --- a/src/exif.cpp +++ b/src/exif.cpp @@ -953,7 +953,7 @@ namespace { long sumToLong(const Exiv2::Exifdatum& md) { long sum = 0; - for (int i = 0; i < md.count(); ++i) { + for (long i = 0; i < md.count(); ++i) { sum += md.toLong(i); } return sum; diff --git a/src/exiv2.cpp b/src/exiv2.cpp index fdf088bf..5c165533 100644 --- a/src/exiv2.cpp +++ b/src/exiv2.cpp @@ -1517,13 +1517,13 @@ namespace { std::string parseEscapes(const std::string& input) { std::string result; - for (unsigned int i = 0; i < input.length(); ++i) { + for (size_t i = 0; i < input.length(); ++i) { char ch = input[i]; if (ch != '\\') { result.push_back(ch); continue; } - int escapeStart = i; + size_t escapeStart = i; if (!(input.length() - 1 > i)) { result.push_back(ch); continue; @@ -1544,7 +1544,7 @@ namespace { result.push_back('\t'); break; case 'u': // Escaping of unicode - if (input.length() - 4 > i) { + if (input.length() >= 4 && input.length() - 4 > i) { int acc = 0; for (int j = 0; j < 4; ++j) { ++i; diff --git a/src/iptc.cpp b/src/iptc.cpp index 4252a988..55bf3d08 100644 --- a/src/iptc.cpp +++ b/src/iptc.cpp @@ -22,6 +22,7 @@ #include "iptc.hpp" #include "types.hpp" #include "error.hpp" +#include "enforce.hpp" #include "value.hpp" #include "datasets.hpp" #include "jpgimage.hpp" @@ -344,7 +345,10 @@ namespace Exiv2 { void IptcData::printStructure(std::ostream& out, const Slice& bytes, uint32_t depth) { - uint32_t i = 0; + if (bytes.size() < 3) { + return; + } + size_t i = 0; while (i < bytes.size() - 3 && bytes.at(i) != 0x1c) i++; depth++; @@ -356,10 +360,12 @@ namespace Exiv2 { char buff[100]; uint16_t record = bytes.at(i + 1); uint16_t dataset = bytes.at(i + 2); + enforce(bytes.size() - i >= 5, kerCorruptedMetadata); uint16_t len = getUShort(bytes.subSlice(i + 3, bytes.size()), bigEndian); snprintf(buff, sizeof(buff), " %6d | %7d | %-24s | %6d | ", record, dataset, Exiv2::IptcDataSets::dataSetName(dataset, record).c_str(), len); + enforce(bytes.size() - i >= 5 + static_cast(len), kerCorruptedMetadata); out << buff << Internal::binaryToString(makeSlice(bytes, i + 5, i + 5 + (len > 40 ? 40 : len))) << (len > 40 ? "..." : "") << std::endl; diff --git a/src/preview.cpp b/src/preview.cpp index de010057..bcfefae7 100644 --- a/src/preview.cpp +++ b/src/preview.cpp @@ -804,7 +804,7 @@ namespace { enforce(size_ <= static_cast(io.size()), kerCorruptedMetadata); DataBuf buf(size_); uint32_t idxBuf = 0; - for (int i = 0; i < sizes.count(); i++) { + for (long i = 0; i < sizes.count(); i++) { uint32_t offset = dataValue.toLong(i); uint32_t size = sizes.toLong(i); enforce(Safe::add(idxBuf, size) < size_, kerCorruptedMetadata); diff --git a/src/tags_int.cpp b/src/tags_int.cpp index 84f9fea9..19cd5203 100644 --- a/src/tags_int.cpp +++ b/src/tags_int.cpp @@ -24,6 +24,7 @@ #include "convert.hpp" #include "error.hpp" +#include "enforce.hpp" #include "i18n.h" // NLS support. #include "canonmn_int.hpp" @@ -2570,7 +2571,7 @@ namespace Exiv2 { { uint16_t bit = 0; uint16_t comma = 0; - for (uint16_t i = 0; i < value.count(); i++ ) { // for each element in value array + for (long i = 0; i < value.count(); i++ ) { // for each element in value array auto bits = static_cast(value.toLong(i)); for (uint16_t b = 0; b < 16; ++b) { // for every bit if (bits & (1 << b)) { diff --git a/src/tiffcomposite_int.cpp b/src/tiffcomposite_int.cpp index 3e13493c..26e189cc 100644 --- a/src/tiffcomposite_int.cpp +++ b/src/tiffcomposite_int.cpp @@ -398,7 +398,7 @@ namespace Exiv2 { return; } uint32_t size = 0; - for (int i = 0; i < pSize->count(); ++i) { + for (long i = 0; i < pSize->count(); ++i) { size += static_cast(pSize->toLong(i)); } auto offset = static_cast(pValue()->toLong(0)); @@ -455,7 +455,7 @@ namespace Exiv2 { #endif return; } - for (int i = 0; i < pValue()->count(); ++i) { + for (long i = 0; i < pValue()->count(); ++i) { const auto offset = static_cast(pValue()->toLong(i)); const byte* pStrip = pData + baseOffset + offset; const auto size = static_cast(pSize->toLong(i)); diff --git a/src/tiffvisitor_int.cpp b/src/tiffvisitor_int.cpp index a24c007c..69a90f11 100644 --- a/src/tiffvisitor_int.cpp +++ b/src/tiffvisitor_int.cpp @@ -456,7 +456,7 @@ namespace Exiv2 { // create vector of signedShorts from unsignedShorts in Exif.Canon.AFInfo std::vector ints; std::vector uint; - for (int i = 0; i < object->pValue()->count(); i++) { + for (long i = 0; i < object->pValue()->count(); i++) { ints.push_back(static_cast(object->pValue()->toLong(i))); uint.push_back(static_cast(object->pValue()->toLong(i))); } @@ -505,10 +505,10 @@ namespace Exiv2 { auto v = Exiv2::Value::create(record.bSigned ? Exiv2::signedShort : Exiv2::unsignedShort); std::ostringstream s; if (record.bSigned) { - for (int16_t k = 0; k < record.size; k++) + for (uint16_t k = 0; k < record.size; k++) s << " " << ints.at(nStart++); } else { - for (int16_t k = 0; k < record.size; k++) + for (uint16_t k = 0; k < record.size; k++) s << " " << uint.at(nStart++); } diff --git a/src/xmp.cpp b/src/xmp.cpp index b00681d9..2e0e9d91 100644 --- a/src/xmp.cpp +++ b/src/xmp.cpp @@ -789,7 +789,7 @@ namespace Exiv2 { if (i.typeId() == xmpBag || i.typeId() == xmpSeq || i.typeId() == xmpAlt) { printNode(ns, i.tagName(), "", options); meta.SetProperty(ns.c_str(), i.tagName().c_str(), nullptr, options); - for (int idx = 0; idx < i.count(); ++idx) { + for (long idx = 0; idx < i.count(); ++idx) { const std::string item = i.tagName() + "[" + toString(idx + 1) + "]"; printNode(ns, item, i.toString(idx), 0); meta.SetProperty(ns.c_str(), item.c_str(), i.toString(idx).c_str()); diff --git a/tests/bugfixes/github/test_issue_ghsa_hqjh_hpv8_8r9p.py b/tests/bugfixes/github/test_issue_ghsa_hqjh_hpv8_8r9p.py new file mode 100644 index 00000000..d6c8d602 --- /dev/null +++ b/tests/bugfixes/github/test_issue_ghsa_hqjh_hpv8_8r9p.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- + +from system_tests import CaseMeta, FileDecoratorBase, path +from struct import * + +# The PoC is a fairly large file, mostly consisting of zero bytes, +# so it would be a waste of storage to check it into the repo. +# Instead, we can generate the PoC with a small amount of code: +class CreatePoC(FileDecoratorBase): + """ + This class copies files from test/data to test/tmp + Copied files are NOT removed in tearDown + Example: @CopyTmpFiles("$data_path/test_issue_1180.exv") + """ + + #: override the name of the file list + FILE_LIST_NAME = '_tmp_files' + + def setUp_file_action(self, expanded_file_name): + size = 0x20040 + contents = pack('<2sI8sHHIIHHII', bytes(b'II'), 14, bytes(b'HEAPCCDR'), \ + 1, 0x300b, size - 26, 12, 1, 0x102a, size - 38, 12) + \ + bytes(bytearray(size-38)) + f = open(expanded_file_name, 'wb') + f.write(contents) + f.close() + + def tearDown_file_action(self, f): + """ + Do nothing. We don't clean up TmpFiles + """ + +# This decorator generates the PoC file. +@CreatePoC("$tmp_path/issue_ghsa_hqjh_hpv8_8r9p_poc.crw") + +class CrwMapDecodeArrayInfiniteLoop(metaclass=CaseMeta): + """ + Regression test for the bug described in: + https://github.com/Exiv2/exiv2/security/advisories/GHSA-hqjh-hpv8-8r9p + """ + url = "https://github.com/Exiv2/exiv2/security/advisories/GHSA-hqjh-hpv8-8r9p" + + filename = path("$tmp_path/issue_ghsa_hqjh_hpv8_8r9p_poc.crw") + + commands = ["$exiv2 $filename"] + stdout = [""] + stderr = [ +"""Exiv2 exception in print action for file $filename: +$kerCorruptedMetadata +"""] + retval = [1]