mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-11-12 17:19:28 +01:00
Archives, caches, HTTP: Bound sizes from headers and servers
- Zip VFS: cap entries at 512 MB (the old 4 GB cap let a 2^32 size wrap the allocation to nothing on 32-bit hosts), fail reads that come up short instead of returning uninitialized memory, and return 0 rather than (size_t)-1 from Read on a corrupt stream, which crashed texture loading. - Zipped disc images are capped at 4 GB and must fit a size_t before we allocate them. - Installing a zipped ISO took the name after the last '/' only, so a name using backslashes could write outside the games folder on Windows. Require a safe filename, and report failures instead of always succeeding (which could delete the source zip after a failed extract). - Disk cache files: reject block sizes we don't create (0 divided by zero), drop index entries that share a block (eviction then never finished), and stop evicting when no progress is possible. Negative or empty reads no longer index the cache out of bounds. - HTTP: reject implausible Content-Length values, and don't cache files in RAM that can't fit (the size wrapped on 32-bit hosts, overflowing the buffer). - Gzip-encoded HTTP responses are capped at 256 MB decompressed. - Directory VFS: don't open directories as files, check Ftell, and bound the case-insensitive index recursion. - Skip zip entries without a name when identifying zips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
382a8eecea
commit
50e9b6691c
12 files changed
+114
-20
No files matched your search
@@ -54,7 +54,7 @@ bool compress_string(const std::string& str, std::string *dest, int compressionl
|
||||
}
|
||||
|
||||
/** Decompress an STL string using zlib and return the original data. */
|
||||
bool decompress_string(const std::string& str, std::string *dest) {
|
||||
bool decompress_string(const std::string& str, std::string *dest, size_t maxSize) {
|
||||
if (!str.size())
|
||||
return false;
|
||||
|
||||
@@ -85,7 +85,11 @@ bool decompress_string(const std::string& str, std::string *dest) {
|
||||
outstring.append(outbuffer,
|
||||
zs.total_out - outstring.size());
|
||||
}
|
||||
|
||||
if (outstring.size() > maxSize) {
|
||||
inflateEnd(&zs);
|
||||
ERROR_LOG(Log::IO, "Decompressed data is larger than %d bytes, giving up", (int)maxSize);
|
||||
return false;
|
||||
}
|
||||
} while (ret == Z_OK);
|
||||
|
||||
inflateEnd(&zs);
|
||||
@@ -95,6 +99,6 @@ bool decompress_string(const std::string& str, std::string *dest) {
|
||||
return false;
|
||||
}
|
||||
|
||||
*dest = outstring;
|
||||
*dest = std::move(outstring);
|
||||
return true;
|
||||
}
|
||||
@@ -4,4 +4,5 @@
|
||||
|
||||
// inflate/deflate convenience wrapper. Uses zlib.
|
||||
bool compress_string(const std::string& str, std::string *dest, int compressionlevel = 9);
|
||||
bool decompress_string(const std::string& str, std::string *dest);
|
||||
// Fails if the output would be larger than maxSize (a small input can expand enormously).
|
||||
bool decompress_string(const std::string& str, std::string *dest, size_t maxSize = 256 * 1024 * 1024);
|
||||
@@ -24,13 +24,17 @@ static std::string ToLowerASCII(std::string_view str) {
|
||||
return lower;
|
||||
}
|
||||
|
||||
void DirectoryReader::IndexDirectory(const Path &dir, const std::string &prefix) {
|
||||
void DirectoryReader::IndexDirectory(const Path &dir, const std::string &prefix, int depth) {
|
||||
// A symlink loop would otherwise recurse until the stack runs out.
|
||||
if (depth > 32) {
|
||||
return;
|
||||
}
|
||||
std::vector<File::FileInfo> files;
|
||||
File::GetFilesInDir(dir, &files);
|
||||
for (const File::FileInfo &file : files) {
|
||||
const std::string relative = prefix + file.name;
|
||||
if (file.isDirectory) {
|
||||
IndexDirectory(file.fullName, relative + "/");
|
||||
IndexDirectory(file.fullName, relative + "/", depth + 1);
|
||||
} else {
|
||||
lowerIndex_[ToLowerASCII(relative)] = relative;
|
||||
}
|
||||
@@ -44,7 +48,7 @@ bool DirectoryReader::FindIgnoringCase(std::string_view path, std::string *actua
|
||||
std::lock_guard<std::mutex> guard(indexLock_);
|
||||
if (!indexBuilt_) {
|
||||
// Only done on the first miss, so a directory where every name matches never pays for it.
|
||||
IndexDirectory(path_, "");
|
||||
IndexDirectory(path_, "", 0);
|
||||
indexBuilt_ = true;
|
||||
INFO_LOG(Log::IO, "Indexed %d files in %s to find names that differ in case", (int)lowerIndex_.size(), path_.c_str());
|
||||
}
|
||||
@@ -134,13 +138,22 @@ void DirectoryReader::ReleaseFile(VFSFileReference *vfsReference) {
|
||||
|
||||
VFSOpenFile *DirectoryReader::OpenFileForRead(VFSFileReference *vfsReference, size_t *size) {
|
||||
DirectoryReaderFileReference *reference = (DirectoryReaderFileReference *)vfsReference;
|
||||
// On some platforms, directories can be opened as files. Their size is meaningless.
|
||||
if (File::IsDirectory(reference->path)) {
|
||||
return nullptr;
|
||||
}
|
||||
FILE *file = File::OpenCFile(reference->path, "rb");
|
||||
if (!file) {
|
||||
return nullptr;
|
||||
}
|
||||
File::Fseek(file, 0, SEEK_END);
|
||||
*size = File::Ftell(file);
|
||||
const int64_t fileSize = File::Ftell(file);
|
||||
File::Fseek(file, 0, SEEK_SET);
|
||||
if (fileSize < 0) {
|
||||
fclose(file);
|
||||
return nullptr;
|
||||
}
|
||||
*size = (size_t)fileSize;
|
||||
DirectoryReaderOpenFile *openFile = new DirectoryReaderOpenFile();
|
||||
openFile->file = file;
|
||||
return openFile;
|
||||
|
||||
@@ -36,7 +36,7 @@ public:
|
||||
private:
|
||||
// Returns the path (relative to path_) of a file whose name matches path except for case.
|
||||
bool FindIgnoringCase(std::string_view path, std::string *actualPath);
|
||||
void IndexDirectory(const Path &dir, const std::string &prefix);
|
||||
void IndexDirectory(const Path &dir, const std::string &prefix, int depth);
|
||||
|
||||
Path path_;
|
||||
bool ignoreCase_ = false;
|
||||
|
||||
@@ -16,8 +16,10 @@
|
||||
#include "Common/StringUtils.h"
|
||||
|
||||
// Sanity cap on a single zip entry's declared uncompressed size, so a corrupt or
|
||||
// malicious zip can't drive an implausible (or, near UINT64_MAX, wrapping) allocation.
|
||||
static constexpr uint64_t MAX_ZIP_ENTRY_SIZE = 1ULL << 32; // 4GB, generous for any real asset.
|
||||
// malicious zip can't drive an implausible allocation (which also has to fit in a
|
||||
// 32-bit size_t). Entries are read whole into memory; the largest real asset, an
|
||||
// uncompressed 8192x8192 replacement texture with mips, is under 512MB.
|
||||
static constexpr uint64_t MAX_ZIP_ENTRY_SIZE = 512ULL * 1024 * 1024;
|
||||
|
||||
ZipContainer::ZipContainer() noexcept : sourceData_(nullptr), zip_(nullptr) {}
|
||||
|
||||
@@ -186,8 +188,13 @@ uint8_t *ZipFileReader::ReadFile(std::string_view path, size_t *size) {
|
||||
return 0;
|
||||
}
|
||||
uint8_t *contents = new uint8_t[zstat.size + 1];
|
||||
zip_fread(file, contents, zstat.size);
|
||||
const zip_int64_t bytesRead = zip_fread(file, contents, zstat.size);
|
||||
zip_fclose(file);
|
||||
if (bytesRead < 0 || (uint64_t)bytesRead != zstat.size) {
|
||||
ERROR_LOG(Log::IO, "Error reading %s from ZIP (%lld of %llu bytes)", temp_path.c_str(), (long long)bytesRead, (unsigned long long)zstat.size);
|
||||
delete[] contents;
|
||||
return 0;
|
||||
}
|
||||
contents[zstat.size] = 0;
|
||||
|
||||
*size = zstat.size;
|
||||
@@ -402,6 +409,13 @@ VFSOpenFile *ZipFileReader::OpenFileForRead(VFSFileReference *vfsReference, size
|
||||
delete openFile;
|
||||
return nullptr;
|
||||
}
|
||||
// Callers allocate the returned size.
|
||||
if (zstat.size > MAX_ZIP_ENTRY_SIZE) {
|
||||
ERROR_LOG(Log::IO, "Zip entry %d claims an implausible size (%llu), refusing to open", reference->zi, (unsigned long long)zstat.size);
|
||||
lock_.unlock();
|
||||
delete openFile;
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
openFile->zf = zip_fopen_index(zip_file_, reference->zi, 0);
|
||||
if (!openFile->zf) {
|
||||
@@ -429,15 +443,20 @@ void ZipFileReader::Rewind(VFSOpenFile *vfsOpenFile) {
|
||||
size_t ZipFileReader::Read(VFSOpenFile *vfsOpenFile, void *buffer, size_t length) {
|
||||
ZipFileReaderOpenFile *file = (ZipFileReaderOpenFile *)vfsOpenFile;
|
||||
_assert_(file);
|
||||
_dbg_assert_(file->zf != nullptr);
|
||||
return zip_fread(file->zf, buffer, length);
|
||||
if (!file->zf) {
|
||||
return 0;
|
||||
}
|
||||
// -1 on errors, like a bad CRC or a corrupt stream.
|
||||
const zip_int64_t bytesRead = zip_fread(file->zf, buffer, length);
|
||||
return bytesRead < 0 ? 0 : (size_t)bytesRead;
|
||||
}
|
||||
|
||||
void ZipFileReader::CloseFile(VFSOpenFile *vfsOpenFile) {
|
||||
ZipFileReaderOpenFile *file = (ZipFileReaderOpenFile *)vfsOpenFile;
|
||||
_assert_(file);
|
||||
_dbg_assert_(file->zf != nullptr);
|
||||
zip_fclose(file->zf);
|
||||
if (file->zf) {
|
||||
zip_fclose(file->zf);
|
||||
}
|
||||
file->zf = nullptr;
|
||||
vfsOpenFile = nullptr;
|
||||
lock_.unlock();
|
||||
|
||||
@@ -87,11 +87,15 @@ size_t DiskCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data,
|
||||
Prepare();
|
||||
size_t readSize;
|
||||
|
||||
if (absolutePos >= filesize_) {
|
||||
if (absolutePos < 0 || absolutePos >= filesize_) {
|
||||
bytes = 0;
|
||||
} else if (absolutePos + (s64)bytes >= filesize_) {
|
||||
bytes = (size_t)(filesize_ - absolutePos);
|
||||
}
|
||||
// The cache computes the last block as pos + bytes - 1.
|
||||
if (bytes == 0) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (cache_ && cache_->IsValid() && (flags & Flags::HINT_UNCACHED) == 0) {
|
||||
readSize = cache_->ReadFromCache(absolutePos, bytes, data);
|
||||
@@ -357,6 +361,7 @@ bool DiskCachingFileLoaderCache::MakeCacheSpaceFor(size_t blocks) {
|
||||
|
||||
while (cacheSize_ > goal) {
|
||||
u16 minGeneration = generation_;
|
||||
bool evicted = false;
|
||||
|
||||
// We increment the iterator inside because we delete things inside.
|
||||
for (size_t i = 0; i < blockIndexLookup_.size(); ++i) {
|
||||
@@ -377,6 +382,7 @@ bool DiskCachingFileLoaderCache::MakeCacheSpaceFor(size_t blocks) {
|
||||
info.generation = 0;
|
||||
info.hits = 0;
|
||||
--cacheSize_;
|
||||
evicted = true;
|
||||
|
||||
// TODO: Doing this in chunks might be a lot better.
|
||||
WriteIndexData(blockIndexLookup_[i], info);
|
||||
@@ -389,6 +395,12 @@ bool DiskCachingFileLoaderCache::MakeCacheSpaceFor(size_t blocks) {
|
||||
}
|
||||
}
|
||||
|
||||
// If we found nothing to evict and the next pass would look for the same generation,
|
||||
// it can't make progress either.
|
||||
if (!evicted && minGeneration == oldestGeneration_) {
|
||||
ERROR_LOG(Log::Loader, "Disk cache: Nothing left to evict, %d blocks counted", (int)cacheSize_);
|
||||
break;
|
||||
}
|
||||
// If we didn't find any, update to the lowest we did find.
|
||||
oldestGeneration_ = minGeneration;
|
||||
}
|
||||
@@ -548,6 +560,9 @@ bool DiskCachingFileLoaderCache::LoadCacheFile(const Path &path) {
|
||||
} else if (header.maxBlocks < MAX_BLOCKS_LOWER_BOUND || header.maxBlocks > MAX_BLOCKS_UPPER_BOUND) {
|
||||
// This means it's not in our safety bounds, reject.
|
||||
valid = false;
|
||||
} else if (header.blockSize != DEFAULT_BLOCK_SIZE) {
|
||||
// We only ever create these, and everything below divides by and allocates the block size.
|
||||
valid = false;
|
||||
}
|
||||
|
||||
// If it's valid, retain the file pointer.
|
||||
@@ -600,6 +615,12 @@ void DiskCachingFileLoaderCache::LoadCacheIndex() {
|
||||
if (index_[i].block == INVALID_BLOCK) {
|
||||
continue;
|
||||
}
|
||||
// Two entries can't share a data block. Keeping both would serve one of them the wrong
|
||||
// data, and count a block that eviction can't find (which then never finishes).
|
||||
if (blockIndexLookup_[index_[i].block] != INVALID_INDEX) {
|
||||
index_[i].block = INVALID_BLOCK;
|
||||
continue;
|
||||
}
|
||||
|
||||
if (index_[i].generation < oldestGeneration_) {
|
||||
oldestGeneration_ = index_[i].generation;
|
||||
|
||||
@@ -89,6 +89,11 @@ void HTTPFileLoader::Prepare() {
|
||||
}
|
||||
if (size_pos != header.npos) {
|
||||
filesize_ = atoll(&header[size_pos]);
|
||||
// The caches size their indexes from this. Nothing we'd load is this big.
|
||||
if (filesize_ < 0 || filesize_ > (64LL << 30)) {
|
||||
ERROR_LOG(Log::Loader, "HTTP: Implausible Content-Length: %s", header.c_str());
|
||||
filesize_ = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (startsWithNoCase(header, "Accept-Ranges:")) {
|
||||
|
||||
@@ -88,6 +88,11 @@ size_t RamCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, F
|
||||
|
||||
void RamCachingFileLoader::InitCache() {
|
||||
std::lock_guard<std::mutex> guard(blocksMutex_);
|
||||
// The whole file has to fit in memory (and a size_t, which may be 32 bits).
|
||||
if (filesize_ <= 0 || (u64)filesize_ > (u64)(SIZE_MAX - BLOCK_SIZE) || (u64)filesize_ > (8ULL << 30)) {
|
||||
ERROR_LOG(Log::IO, "Can't cache a file of %lld bytes in RAM, will fall back to regular reads.", (long long)filesize_);
|
||||
return;
|
||||
}
|
||||
u32 blockCount = (u32)((filesize_ + BLOCK_SIZE - 1) >> BLOCK_SHIFT);
|
||||
// Overallocate for the last block.
|
||||
cache_ = (u8 *)malloc((size_t)blockCount << BLOCK_SHIFT);
|
||||
|
||||
@@ -74,6 +74,12 @@ bool ZipFileLoader::Initialize(int fileIndex) {
|
||||
}
|
||||
|
||||
_dbg_assert_(zstat.index == fileIndex);
|
||||
// The whole file is decompressed into memory. No disc image is anywhere near 4GB, and the
|
||||
// size has to fit in a (possibly 32-bit) size_t too.
|
||||
if (!(zstat.valid & ZIP_STAT_SIZE) || zstat.size > (4ULL << 30) || zstat.size > (u64)SIZE_MAX) {
|
||||
ERROR_LOG(Log::IO, "File index %d in ZIP has an implausible size (%llu)", fileIndex, (unsigned long long)zstat.size);
|
||||
return false;
|
||||
}
|
||||
dataFileSize_ = zstat.size;
|
||||
dataFile_ = zip_fopen_index(zipArchive_, zstat.index, ZIP_FL_UNCHANGED);
|
||||
if (!dataFile_) {
|
||||
|
||||
@@ -92,6 +92,10 @@ size_t BlobFileSystem::SeekFile(u32 handle, s32 position, FileMove type) {
|
||||
entry->second = fileLoader_->FileSize() + position;
|
||||
break;
|
||||
}
|
||||
// Reads take the position as a file offset.
|
||||
if (entry->second < 0) {
|
||||
entry->second = 0;
|
||||
}
|
||||
return (size_t)entry->second;
|
||||
}
|
||||
return 0;
|
||||
|
||||
@@ -678,6 +678,10 @@ void DetectZipFileContents(zip_t *z, ZipFileInfo *info) {
|
||||
// is quite convenient and makes it easy to add shared heuristics.
|
||||
for (int i = 0; i < numFiles; i++) {
|
||||
const char *fn = zip_get_name(z, i, 0);
|
||||
if (!fn) {
|
||||
// A corrupt central directory entry.
|
||||
continue;
|
||||
}
|
||||
|
||||
zip_stat_t stat{};
|
||||
zip_stat_index(z, i, 0, &stat);
|
||||
|
||||
@@ -912,7 +912,8 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
|
||||
return false;
|
||||
}
|
||||
std::string fn = fnPtr;
|
||||
size_t nameOffset = fn.rfind('/');
|
||||
// Zips made on Windows can use either separator.
|
||||
size_t nameOffset = fn.find_last_of("/\\");
|
||||
if (nameOffset == std::string::npos) {
|
||||
nameOffset = 0;
|
||||
} else {
|
||||
@@ -934,6 +935,13 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
|
||||
name = name.substr(2);
|
||||
}
|
||||
|
||||
// The name is used as a filename in the destination, so mustn't be able to point outside it.
|
||||
if (!IsSafePathComponent(name)) {
|
||||
ERROR_LOG(Log::IO, "Bad ISO filename in zip: '%s'", fn.c_str());
|
||||
SetInstallError(I18NCat::INSTALLZIP, "Zip archive corrupt");
|
||||
return false;
|
||||
}
|
||||
|
||||
Path outputISOFilename = destDir;
|
||||
if (outputISOFilename.empty()) {
|
||||
outputISOFilename = Path(g_Config.currentDirectory);
|
||||
@@ -952,8 +960,12 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
|
||||
|
||||
installProgress_ = 1.0f;
|
||||
InstallDone();
|
||||
ResetInstallError();
|
||||
return true;
|
||||
if (success) {
|
||||
ResetInstallError();
|
||||
} else {
|
||||
SetInstallError(I18NCat::INSTALLZIP, "Installation failed");
|
||||
}
|
||||
return success;
|
||||
}
|
||||
|
||||
bool GameManager::InstallZippedPS1Disc(struct zip *z, const std::vector<std::string> &files, const Path &destDir) {
|
||||
|
||||
Reference in new issue
Block a user