diff --git a/Zip/include/Poco/Zip/Decompress.h b/Zip/include/Poco/Zip/Decompress.h index 83b959c2e..5cf7323b2 100644 --- a/Zip/include/Poco/Zip/Decompress.h +++ b/Zip/include/Poco/Zip/Decompress.h @@ -55,9 +55,10 @@ public: ~Decompress() override; /// Destroys the Decompress. - ZipArchive decompressAllFiles(); + ZipArchive decompressAllFiles(const bool checkConsistency = true); /// Decompresses all files stored in the zip File. Can only be called once per Decompress object. /// Use mapping to retrieve the location of the decompressed files + /// if checkConsistency is set to false, archive won't be checked for consistency before decompression bool handleZipEntry(std::istream& zipStream, const ZipLocalFileHeader& hdr) override; diff --git a/Zip/include/Poco/Zip/ZipArchive.h b/Zip/include/Poco/Zip/ZipArchive.h index b6446886f..8c194ca6a 100644 --- a/Zip/include/Poco/Zip/ZipArchive.h +++ b/Zip/include/Poco/Zip/ZipArchive.h @@ -52,6 +52,9 @@ public: ~ZipArchive(); /// Destroys the ZipArchive. + void checkConsistency(); + /// Check archive's consistency + FileInfos::const_iterator fileInfoBegin() const; FileInfos::const_iterator fileInfoEnd() const; diff --git a/Zip/src/Decompress.cpp b/Zip/src/Decompress.cpp index 605ad7227..7075516e1 100644 --- a/Zip/src/Decompress.cpp +++ b/Zip/src/Decompress.cpp @@ -64,10 +64,14 @@ Decompress::~Decompress() } -ZipArchive Decompress::decompressAllFiles() +ZipArchive Decompress::decompressAllFiles(const bool checkConsistency) { poco_assert (_mapping.empty()); ZipArchive arch(_in, *this); + + if (checkConsistency) + arch.checkConsistency(); + return arch; } diff --git a/Zip/src/ZipArchive.cpp b/Zip/src/ZipArchive.cpp index 454544b3b..ebc500de7 100644 --- a/Zip/src/ZipArchive.cpp +++ b/Zip/src/ZipArchive.cpp @@ -14,6 +14,7 @@ #include "Poco/Zip/ZipArchive.h" #include "Poco/Zip/SkipCallback.h" +#include "Poco/Zip/ZipException.h" #include "Poco/Exception.h" #include @@ -62,6 +63,32 @@ ZipArchive::~ZipArchive() } +void ZipArchive::checkConsistency() +{ + for (const auto& [filename, dirHeader]: _infos) + { + Poco::UInt32 dirCRC = dirHeader.getCRC(); + Poco::UInt64 dirUncompressedSize = dirHeader.getUncompressedSize(); + + const auto dataIt = _entries.find(filename); + if (dataIt != _entries.end()) + { + const ZipLocalFileHeader &dataHeader = dataIt->second; + + Poco::UInt32 dataCRC = dataHeader.getCRC(); + Poco::UInt64 dataUncompressedSize = dataHeader.getUncompressedSize(); + + if (dataCRC != dirCRC) + throw ZipException("CRC-32 mismatch: ", filename); + + if (dataUncompressedSize != dirUncompressedSize) + throw ZipException("Uncompressed size mismatch: ", filename); + } + } + +} + + void ZipArchive::parse(std::istream& in, ParseCallback& pc) { // read 4 bytes diff --git a/Zip/testsuite/data/consistency-crc32.zip b/Zip/testsuite/data/consistency-crc32.zip new file mode 100644 index 000000000..0872da562 Binary files /dev/null and b/Zip/testsuite/data/consistency-crc32.zip differ diff --git a/Zip/testsuite/data/consistency-uncompressed.zip b/Zip/testsuite/data/consistency-uncompressed.zip new file mode 100644 index 000000000..ff034f867 Binary files /dev/null and b/Zip/testsuite/data/consistency-uncompressed.zip differ diff --git a/Zip/testsuite/src/ZipTest.cpp b/Zip/testsuite/src/ZipTest.cpp index e16f3da15..61b48570d 100644 --- a/Zip/testsuite/src/ZipTest.cpp +++ b/Zip/testsuite/src/ZipTest.cpp @@ -11,6 +11,7 @@ #include "ZipTest.h" #include "Poco/Zip/SkipCallback.h" #include "Poco/Zip/ZipLocalFileHeader.h" +#include "Poco/Zip/ZipException.h" #include "Poco/Zip/ZipArchive.h" #include "Poco/Zip/ZipStream.h" #include "Poco/Zip/Decompress.h" @@ -287,6 +288,80 @@ void ZipTest::testDecompressZip64() } +void ZipTest::testDecompressConsistency() +{ + // test archives with borked file headers, but correct directory + const std::string testArchives[] = + { + "consistency-crc32.zip", + "consistency-uncompressed.zip" + }; + + + for (const std::string &archive : testArchives) + { + // + // test decompressing all files + // + { + Poco::FileInputStream inputStream(getTestFile("data", archive)); + assertTrue (inputStream.good()); + + Decompress dec(inputStream, Poco::Path::temp()); + + try { + dec.decompressAllFiles(); + assertTrue (false); + } + catch (const ZipException &e) + { + } + } + + + // + // test decompressing all files (ignore consistency check) + // + { + Poco::FileInputStream inputStream(getTestFile("data", archive)); + assertTrue (inputStream.good()); + + Decompress dec(inputStream, Poco::Path::temp()); + + // should not throw since consistency checks are ignored + try { + dec.decompressAllFiles(false); + } + catch (const ZipException &e) + { + assertTrue (false); + } + } + + + // + // test decompressing single file + // + { + Poco::FileInputStream inputStream(getTestFile("data", archive)); + assertTrue (inputStream.good()); + + ZipArchive archive(inputStream); + + // File decompression code is skipped since + // we are not expecting archive to be processed further + try + { + archive.checkConsistency(); + assertTrue (false); + } + catch (const ZipException &e) + { + } + } + }; +} + void ZipTest::testValidPath() { assertTrue (ZipCommon::isValidPath(".")); @@ -357,6 +432,7 @@ CppUnit::Test* ZipTest::suite() CppUnit_addTest(pSuite, ZipTest, testCrcAndSizeAfterDataEncapsulated); CppUnit_addTest(pSuite, ZipTest, testDecompressZip64); CppUnit_addTest(pSuite, ZipTest, testValidPath); + CppUnit_addTest(pSuite, ZipTest, testDecompressConsistency); return pSuite; } diff --git a/Zip/testsuite/src/ZipTest.h b/Zip/testsuite/src/ZipTest.h index cd77baaae..fa4971525 100644 --- a/Zip/testsuite/src/ZipTest.h +++ b/Zip/testsuite/src/ZipTest.h @@ -29,6 +29,7 @@ public: void testDecompressSingleFile(); void testDecompressSingleFileInDir(); void testDecompress(); + void testDecompressConsistency(); void testDecompressFlat(); void testDecompressVuln(); void testDecompressFlatVuln();