Index: ps/trunk/source/lib/file/archive/archive_zip.cpp =================================================================== --- ps/trunk/source/lib/file/archive/archive_zip.cpp +++ ps/trunk/source/lib/file/archive/archive_zip.cpp @@ -269,6 +269,11 @@ cd_size = (size_t)read_le32(&m_cd_size); } + off_t getCommentLength() const + { + return static_cast(read_le16(&m_comment_len)); + } + private: u32 m_magic; u16 m_diskNum; @@ -521,8 +526,40 @@ io::Operation op(*file.get(), buf, scanSize, ofs); RETURN_STATUS_IF_ERR(io::Run(op)); - // look for ECDR in buffer - const ECDR* ecdr = (const ECDR*)FindRecord(buf, scanSize, buf, ecdr_magic, sizeof(ECDR)); + // Scanning for ECDR first assumes no comment exists + // (standard case), so ECDR structure exists right at + // end of file + off_t offsetInBlock = scanSize - sizeof(ECDR); + const ECDR* ecdr = nullptr; + + for (off_t commentSize = 0; commentSize <= offsetInBlock && !ecdr; ++commentSize) + { + const u8 *pECDRTest = buf + offsetInBlock - commentSize; + if (*(const u32*)pECDRTest == ecdr_magic) + { + // Signature matches, test whether comment + // fills up the whole space following the + // ECDR + ecdr = (const ECDR*)pECDRTest; + if (commentSize != ecdr->getCommentLength()) + { + // Signature matches but there is some other data between + // header, comment and EOF. There are three possibilities + // for this: + // 1) Header file format and size differ from what we expect + // 2) File has been truncated + // 3) The magic id occurs inside a zip comment + ecdr = nullptr; + } + else + { + // Seems like a valid archive header before an archive-level + // comment + break; + } + } + } + if(!ecdr) return INFO::CANNOT_HANDLE; @@ -534,17 +571,10 @@ { const size_t maxScanSize = 66000u; // see below io::BufferPtr buf(io::Allocate(maxScanSize)); - - // expected case: ECDR at EOF; no file comment - Status ret = ScanForEcdr(file, fileSize, buf.get(), sizeof(ECDR), cd_numEntries, cd_ofs, cd_size); - if(ret == INFO::OK) - return INFO::OK; - // worst case: ECDR precedes 64 KiB of file comment - ret = ScanForEcdr(file, fileSize, buf.get(), maxScanSize, cd_numEntries, cd_ofs, cd_size); + Status ret = ScanForEcdr(file, fileSize, static_cast(buf.get()), maxScanSize, cd_numEntries, cd_ofs, cd_size); if(ret == INFO::OK) return INFO::OK; - // both ECDR scans failed - this is not a valid Zip file. io::Operation op(*file.get(), buf.get(), sizeof(LFH)); RETURN_STATUS_IF_ERR(io::Run(op)); // the Zip file has an LFH but lacks an ECDR. this can happen if Index: ps/trunk/source/lib/file/archive/tests/test_archive_zip.h =================================================================== --- ps/trunk/source/lib/file/archive/tests/test_archive_zip.h +++ ps/trunk/source/lib/file/archive/tests/test_archive_zip.h @@ -0,0 +1,101 @@ +/* Copyright (C) 2020 Wildfire Games. + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include "lib/self_test.h" +#include "lib/file/file_system.h" // For CreateDirectories() +#include "lib/file/archive/archive_zip.h" + +#include "lib/file/io/io.h" // For io::Store() + +class TestArchiveZip : public CxxTest::TestSuite +{ +public: + void test_scan_suspiciousZipFile() + { + OsPath testDir = + DataDir() / "mods" / "_test.lib" / + "file" / "archive"; + OsPath testPath = testDir / "test.zip"; + + // Hexdump of a zip archive with a comment at the end + const unsigned char cbTestZip[] = { + 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x00, 0x00, 0x08, 0x00, 0x5a, 0xb6, 0xba, 0x4c, 0xf4, 0x84, + 0x59, 0xea, 0x70, 0x00, 0x00, 0x00, 0xae, 0x00, 0x00, 0x00, 0x16, 0x00, 0x1c, 0x00, 0x62, 0x75, + 0x69, 0x6c, 0x64, 0x7a, 0x69, 0x70, 0x77, 0x69, 0x74, 0x68, 0x63, 0x6f, 0x6d, 0x6d, 0x65, 0x6e, + 0x74, 0x2e, 0x73, 0x68, 0x55, 0x54, 0x09, 0x00, 0x03, 0xac, 0xc8, 0x09, 0x5b, 0xaf, 0xc8, 0x09, + 0x5b, 0x75, 0x78, 0x0b, 0x00, 0x01, 0x04, 0xe8, 0x03, 0x00, 0x00, 0x04, 0x64, 0x00, 0x00, 0x00, + 0x4d, 0x8d, 0x5d, 0x0a, 0x83, 0x30, 0x0c, 0xc7, 0xdf, 0x7b, 0x8a, 0xa8, 0x6f, 0x03, 0xdb, 0x0b, + 0x0c, 0x4f, 0xe1, 0x05, 0x5a, 0xcd, 0xd6, 0x80, 0x26, 0x65, 0xc9, 0x10, 0x76, 0xfa, 0x55, 0x64, + 0x74, 0x6f, 0x3f, 0xfe, 0x9f, 0x43, 0x17, 0x12, 0x71, 0xd0, 0xec, 0x3e, 0x54, 0xc0, 0x50, 0xcd, + 0x9f, 0x70, 0xf3, 0x55, 0xf1, 0xa1, 0x22, 0x8b, 0x61, 0xd3, 0xa7, 0x0b, 0x6d, 0x2f, 0x0e, 0x97, + 0x2c, 0xd0, 0xcf, 0x99, 0x14, 0x1e, 0xb4, 0x21, 0x2c, 0xc2, 0x16, 0x89, 0x15, 0x22, 0xfc, 0x6a, + 0x2b, 0xee, 0xc2, 0x6a, 0xaf, 0x68, 0x24, 0x0c, 0x07, 0x59, 0xae, 0x66, 0x11, 0x55, 0x4a, 0xb5, + 0x90, 0xde, 0xcf, 0x1e, 0xa6, 0xbf, 0xc5, 0xf6, 0x37, 0x1e, 0xed, 0xf2, 0xde, 0x02, 0xee, 0x0b, + 0x50, 0x4b, 0x01, 0x02, 0x1e, 0x03, 0x14, 0x00, 0x00, 0x00, 0x08, 0x00, 0x5a, 0xb6, 0xba, 0x4c, + 0xf4, 0x84, 0x59, 0xea, 0x70, 0x00, 0x00, 0x00, 0xae, 0x00, 0x00, 0x00, 0x16, 0x00, 0x18, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xed, 0x81, 0x00, 0x00, 0x00, 0x00, 0x62, 0x75, + 0x69, 0x6c, 0x64, 0x7a, 0x69, 0x70, 0x77, 0x69, 0x74, 0x68, 0x63, 0x6f, 0x6d, 0x6d, 0x65, 0x6e, + 0x74, 0x2e, 0x73, 0x68, 0x55, 0x54, 0x05, 0x00, 0x03, 0xac, 0xc8, 0x09, 0x5b, 0x75, 0x78, 0x0b, + 0x00, 0x01, 0x04, 0xe8, 0x03, 0x00, 0x00, 0x04, 0x64, 0x00, 0x00, 0x00, 0x50, 0x4b, 0x05, 0x06, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x5c, 0x00, 0x00, 0x00, 0xc0, 0x00, 0x00, 0x00, + 0x3e, 0x00, 0x54, 0x68, 0x69, 0x73, 0x20, 0x66, 0x69, 0x6c, 0x65, 0x20, 0x63, 0x6f, 0x6e, 0x74, + 0x61, 0x69, 0x6e, 0x73, 0x20, 0x61, 0x20, 0x7a, 0x69, 0x70, 0x6e, 0x6f, 0x74, 0x65, 0x20, 0x64, + 0x65, 0x6d, 0x6f, 0x6e, 0x73, 0x74, 0x72, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x20, 0x77, 0x69, 0x74, + 0x68, 0x20, 0x61, 0x20, 0x70, 0x6f, 0x73, 0x73, 0x69, 0x62, 0x6c, 0x65, 0x20, 0x62, 0x75, 0x67}; + + // HACK: This test case requires a sample zip archive with + // a comment at the end. Since SVN patches cannot + // contain binary data, the sample archive is stored + // as a hexdump above and written to the file system + // as the test executes. + CreateDirectories(testDir, 0700, false); + // Note: This file write access has to be done synchronously, + // as the following statement expects the file to be + // present and complete + io::Store(testPath, cbTestZip, sizeof(cbTestZip)/sizeof(cbTestZip[0])); + + PIArchiveReader testee = CreateArchiveReader_Zip(testPath); + + TS_ASSERT(nullptr != testee); + + resultbuffer = ""; + testee->ReadEntries(TestArchiveZip::ArchiveEntryCallback, 0); + + TS_ASSERT_EQUALS("buildzipwithcomment.sh", resultbuffer); + } + +private: + static std::string resultbuffer; + + static void ArchiveEntryCallback( + const VfsPath& path, + const CFileInfo& UNUSED(fileInfo), + PIArchiveFile UNUSED(archiveFile), + uintptr_t UNUSED(cbData)) + { + resultbuffer = path.string8(); + } +}; + +// Implementation of the static buffer used to communicate with ArchiveEntryCallback +std::string TestArchiveZip::resultbuffer; +