mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-01 14:58:14 +00:00
ZipFileReader: guard against implausible/overflowing declared sizes
ReadFile()/ReadSingleFileFromZip() allocated/resized directly off a zip entry's declared uncompressed size with no sanity check. A crafted size near UINT64_MAX would wrap ReadFile()'s "size + 1" to 0, allocating almost nothing while zip_fread() still writes the full declared size into it - a length-field-driven heap overflow from a malicious zip/texture pack. Both now reject entries above a generous 4GB cap. Also fixes GetFileInfo() reading zstat.name[strlen(name)-1] unchecked, which underflows to SIZE_MAX for a zero-length entry name. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
This commit is contained in:
1 parent
99ab8b81ca
commit
6ee6641fe5
1 file changed
+16
-1
@@ -15,6 +15,10 @@
|
||||
#include "Common/File/VFS/ZipFileReader.h"
|
||||
#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.
|
||||
|
||||
ZipContainer::ZipContainer() noexcept : sourceData_(nullptr), zip_(nullptr) {}
|
||||
|
||||
ZipContainer::ZipContainer(const Path &path) : sourceData_(new SourceData {path, nullptr}), zip_(nullptr) {
|
||||
@@ -169,6 +173,13 @@ uint8_t *ZipFileReader::ReadFile(std::string_view path, size_t *size) {
|
||||
ERROR_LOG(Log::IO, "Error opening %s from ZIP", temp_path.c_str());
|
||||
return 0;
|
||||
}
|
||||
// Sanity check the declared size before trusting it for an allocation - a corrupt
|
||||
// or malicious zip could claim a huge (even ~UINT64_MAX, which would wrap the +1
|
||||
// below to 0) size here.
|
||||
if (zstat.size > MAX_ZIP_ENTRY_SIZE) {
|
||||
ERROR_LOG(Log::IO, "Zip entry %s claims an implausible size (%llu), refusing to read", temp_path.c_str(), (unsigned long long)zstat.size);
|
||||
return 0;
|
||||
}
|
||||
zip_file *file = zip_fopen_index(zip_file_, zstat.index, ZIP_FL_NOCASE | ZIP_FL_UNCHANGED);
|
||||
if (!file) {
|
||||
ERROR_LOG(Log::IO, "Error opening %s from ZIP", temp_path.c_str());
|
||||
@@ -311,7 +322,7 @@ bool ZipFileReader::GetFileInfo(std::string_view path, File::FileInfo *info) {
|
||||
}
|
||||
|
||||
// Zips usually don't contain directory entries, but they may.
|
||||
if ((zstat.valid & ZIP_STAT_NAME) != 0 && zstat.name) {
|
||||
if ((zstat.valid & ZIP_STAT_NAME) != 0 && zstat.name && zstat.name[0] != '\0') {
|
||||
info->isDirectory = zstat.name[strlen(zstat.name) - 1] == '/';
|
||||
}
|
||||
if ((zstat.valid & ZIP_STAT_SIZE) != 0) {
|
||||
@@ -436,6 +447,10 @@ bool ReadSingleFileFromZip(Path zipFile, const char *path, std::string *data, st
|
||||
if (zip_stat(zip, path, ZIP_FL_NOCASE | ZIP_FL_UNCHANGED, &zstat) != 0) {
|
||||
return false;
|
||||
}
|
||||
if (zstat.size > MAX_ZIP_ENTRY_SIZE) {
|
||||
ERROR_LOG(Log::IO, "Zip entry %s claims an implausible size (%llu), refusing to read", path, (unsigned long long)zstat.size);
|
||||
return false;
|
||||
}
|
||||
zip_file *file = zip_fopen_index(zip, zstat.index, ZIP_FL_UNCHANGED);
|
||||
if (!file) {
|
||||
return false;
|
||||
|
||||
Reference in new issue
Block a user