Index: source/lib/file/archive/archive.h =================================================================== --- source/lib/file/archive/archive.h +++ source/lib/file/archive/archive.h @@ -55,7 +55,7 @@ * called for each archive entry. * @param pathname full pathname of entry; only valid during the callback. **/ - typedef void (*ArchiveEntryCallback)(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData); + typedef Status (*ArchiveEntryCallback)(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData); virtual Status ReadEntries(ArchiveEntryCallback cb, uintptr_t cbData) = 0; }; Index: source/lib/file/archive/archive_zip.cpp =================================================================== --- source/lib/file/archive/archive_zip.cpp +++ source/lib/file/archive/archive_zip.cpp @@ -27,9 +27,6 @@ #include "precompiled.h" #include "lib/file/archive/archive_zip.h" -#include -#include - #include "lib/utf8.h" #include "lib/bits.h" #include "lib/byte_order.h" @@ -41,6 +38,10 @@ #include "lib/file/file.h" #include "lib/file/io/io.h" +#include +#include +#include + //----------------------------------------------------------------------------- // timestamp conversion: DOS FAT <-> Unix time_t //----------------------------------------------------------------------------- @@ -128,9 +129,14 @@ memcpy((char*)this + sizeof(LFH), pathnameUTF8.c_str(), pathnameSize); } - size_t Size() const + bool IsMagicValid() const { - ENSURE(m_magic == lfh_magic); + return m_magic == lfh_magic; + } + + size_t GetSize() const + { + ENSURE(IsMagicValid()); size_t size = sizeof(LFH); size += read_le16(&m_fn_len); size += read_le16(&m_e_len); @@ -321,7 +327,7 @@ virtual Status Load(const OsPath& UNUSED(name), const std::shared_ptr& buf, size_t size) const { - AdjustOffset(); + RETURN_STATUS_IF_ERR(AdjustOffset()); PICodec codec; switch(m_method) @@ -333,7 +339,7 @@ codec = CreateDecompressor_ZLibDeflate(); break; default: - WARN_RETURN(ERR::ARCHIVE_UNKNOWN_METHOD); + RETURN_STATUS_IF_ERR(ERR::ARCHIVE_UNKNOWN_METHOD); } Stream stream(codec); @@ -343,7 +349,8 @@ RETURN_STATUS_IF_ERR(io::Run(op, io::Parameters(), streamFeeder)); RETURN_STATUS_IF_ERR(stream.Finish()); #if CODEC_COMPUTE_CHECKSUM - ENSURE(m_checksum == stream.Checksum()); + if (m_checksum != stream.Checksum()) + return ERR::CORRUPTED; #endif return INFO::OK; @@ -403,10 +410,10 @@ * reduce seeks: since reading the file will typically follow, the * block cache entirely absorbs the IO cost. **/ - void AdjustOffset() const + Status AdjustOffset() const { if(!(m_flags & NeedsFixup)) - return; + return INFO::OK; m_flags &= ~NeedsFixup; // performance note: this ends up reading one file block, which is @@ -414,8 +421,11 @@ // previously read file (i.e. both are small). LFH lfh; io::Operation op(*m_file.get(), 0, sizeof(LFH), m_ofs); - if(io::Run(op, io::Parameters(), LFH_Copier((u8*)&lfh, sizeof(LFH))) == INFO::OK) - m_ofs += (off_t)lfh.Size(); + RETURN_STATUS_IF_ERR(io::Run(op, io::Parameters(), LFH_Copier((u8*)&lfh, sizeof(LFH)))); + if (!lfh.IsMagicValid()) + return ERR::ARCHIVE_UNKNOWN_FORMAT; + m_ofs += (off_t)lfh.GetSize(); + return INFO::OK; } PFile m_file; @@ -463,9 +473,10 @@ for(size_t i = 0; i < cd_numEntries; i++) { // scan for next CDFH - CDFH* cdfh = (CDFH*)FindRecord(buf.get(), cd_size, pos, cdfh_magic, sizeof(CDFH)); + auto [pointer, status] = FindRecord(buf.get(), cd_size, pos, cdfh_magic, sizeof(CDFH)); + CDFH* cdfh = reinterpret_cast(const_cast(pointer)); if(!cdfh) - WARN_RETURN(ERR::CORRUPTED); + return ERR::CORRUPTED; const Path relativePathname(cdfh->Pathname()); if(!relativePathname.IsDirectory()) @@ -473,7 +484,7 @@ const OsPath name = relativePathname.Filename(); CFileInfo fileInfo(name, cdfh->USize(), cdfh->MTime()); std::shared_ptr archiveFile = std::make_shared(m_file, cdfh->HeaderOffset(), cdfh->CSize(), cdfh->Checksum(), cdfh->Method()); - cb(relativePathname, fileInfo, archiveFile, cbData); + RETURN_STATUS_IF_ERR(cb(relativePathname, fileInfo, archiveFile, cbData)); } pos += cdfh->Size(); @@ -493,7 +504,7 @@ * @param recordSize size of record (including signature) * @return pointer to record within buffer or 0 if not found. **/ - static const u8* FindRecord(const u8* buf, size_t size, const u8* start, u32 magic, size_t recordSize) + static std::tuple FindRecord(const u8* buf, size_t size, const u8* start, u32 magic, size_t recordSize) { // (don't use as the counter - otherwise we can't tell if // scanning within the buffer was necessary.) @@ -502,15 +513,19 @@ // found it if(*(u32*)p == magic) { - ENSURE(p == start); // otherwise, the archive is a bit broken - return p; + if (p != start) + { + // The archive is a bit broken. + return {nullptr, ERR::CORRUPTED}; + } + return {p, INFO::OK}; } } // passed EOF, didn't find it. // note: do not warn - this happens in the initial ECDR search at // EOF if the archive contains a comment field. - return 0; + return {nullptr, INFO::OK}; } // search for ECDR in the last bytes of the file. @@ -585,11 +600,13 @@ // because it'd be slow. // - do not warn - the corrupt archive will be deleted on next // successful archive builder run anyway. - if(FindRecord(buf.get(), sizeof(LFH), buf.get(), lfh_magic, sizeof(LFH))) - return ERR::CORRUPTED; // NOWARN + auto [pointer, status] = FindRecord(buf.get(), sizeof(LFH), buf.get(), lfh_magic, sizeof(LFH)); + RETURN_STATUS_IF_ERR(status); + if(pointer) + return ERR::CORRUPTED; // totally bogus else - WARN_RETURN(ERR::ARCHIVE_UNKNOWN_FORMAT); + return ERR::ARCHIVE_UNKNOWN_FORMAT; } PFile m_file; Index: source/lib/file/archive/tests/test_archive_zip.h =================================================================== --- source/lib/file/archive/tests/test_archive_zip.h +++ source/lib/file/archive/tests/test_archive_zip.h @@ -104,12 +104,13 @@ } private: - static void ArchiveEntryCallback( + static Status ArchiveEntryCallback( const VfsPath& path, const CFileInfo& UNUSED(fileInfo), PIArchiveFile UNUSED(archiveFile), uintptr_t UNUSED(cbData)) { g_ResultBuffer = path.string8(); + return INFO::OK; } }; Index: source/lib/file/vfs/vfs_populate.cpp =================================================================== --- source/lib/file/vfs/vfs_populate.cpp +++ source/lib/file/vfs/vfs_populate.cpp @@ -85,7 +85,7 @@ m_directory->AddFile(file); } - static void AddArchiveFile(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData) + static Status AddArchiveFile(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData) { PopulateHelper* this_ = (PopulateHelper*)cbData; @@ -93,7 +93,7 @@ // don't always place directory entries before their files) const size_t flags = VFS_LOOKUP_ADD|VFS_LOOKUP_SKIP_POPULATE; VfsDirectory* directory; - WARN_IF_ERR(vfs_Lookup(pathname, this_->m_directory, directory, 0, flags)); + RETURN_STATUS_IF_ERR(vfs_Lookup(pathname, this_->m_directory, directory, 0, flags)); const VfsPath name = fileInfo.Name(); const VfsFile file(name, (size_t)fileInfo.Size(), fileInfo.MTime(), this_->m_realDirectory->Priority(), archiveFile); @@ -101,10 +101,12 @@ { directory->DeleteSubtree(file); if(!(this_->m_realDirectory->Flags() & VFS_MOUNT_KEEP_DELETED)) - return; + return INFO::OK; } directory->AddFile(file); + + return INFO::OK; } Status AddFiles(const CFileInfos& files) const Index: source/ps/SavedGame.cpp =================================================================== --- source/ps/SavedGame.cpp +++ source/ps/SavedGame.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2021 Wildfire Games. +/* Copyright (C) 2022 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -31,6 +31,7 @@ #include "ps/Game.h" #include "ps/Mod.h" #include "ps/Pyrogenesis.h" +#include "ps/Util.h" #include "scriptinterface/Object.h" #include "scriptinterface/JSON.h" #include "scriptinterface/StructuredClone.h" @@ -171,25 +172,26 @@ m_Metadata.init(rq.cx); } - static void ReadEntryCallback(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData) + static Status ReadEntryCallback(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile, uintptr_t cbData) { - ((CGameLoader*)cbData)->ReadEntry(pathname, fileInfo, archiveFile); + return ((CGameLoader*)cbData)->ReadEntry(pathname, fileInfo, archiveFile); } - void ReadEntry(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile) + Status ReadEntry(const VfsPath& pathname, const CFileInfo& fileInfo, PIArchiveFile archiveFile) { if (pathname == L"metadata.json") { std::string buffer; buffer.resize(fileInfo.Size()); - WARN_IF_ERR(archiveFile->Load("", DummySharedPtr((u8*)buffer.data()), buffer.size())); + RETURN_STATUS_IF_ERR(archiveFile->Load("", DummySharedPtr((u8*)buffer.data()), buffer.size())); Script::ParseJSON(ScriptRequest(m_ScriptInterface), buffer, &m_Metadata); } else if (pathname == L"simulation.dat" && m_SavedState) { m_SavedState->resize(fileInfo.Size()); - WARN_IF_ERR(archiveFile->Load("", DummySharedPtr((u8*)m_SavedState->data()), m_SavedState->size())); + RETURN_STATUS_IF_ERR(archiveFile->Load("", DummySharedPtr((u8*)m_SavedState->data()), m_SavedState->size())); } + return INFO::OK; } JS::Value GetMetadata() @@ -260,11 +262,11 @@ continue; // skip this file } - CGameLoader loader(scriptInterface, NULL); + CGameLoader loader(scriptInterface, nullptr); err = archiveReader->ReadEntries(CGameLoader::ReadEntryCallback, (uintptr_t)&loader); - if (err < 0) + if (err != INFO::OK) { - DEBUG_WARN_ERR(err); + LOGWARNING("Failed to process saved game '%s': %s", realPath.string8(), GetStatusAsString(err).c_str()); continue; // skip this file } JS::RootedValue metadata(rq.cx, loader.GetMetadata());