diff --git a/Core/FileSystems/ISOFileSystem.cpp b/Core/FileSystems/ISOFileSystem.cpp index 20994af8b4..e3a6ad68b4 100644 --- a/Core/FileSystems/ISOFileSystem.cpp +++ b/Core/FileSystems/ISOFileSystem.cpp @@ -302,6 +302,20 @@ void ISOFileSystem::ReadDirectory(TreeEntry *root) const { ERROR_LOG(Log::FileSystem, "File '%s' starts or ends outside ISO. firstDataSector: %d len: %d", entry->BuildPath().c_str(), (int)dir.firstDataSector, (int)dir.dataLength); } + // The directory record is untrusted, and callers size host buffers from entry->size, so + // don't let it claim more data than the image actually contains. We clamp rather than + // drop the entry - truncated ISOs are common and used to work with just the warning + // above, and dropping EBOOT.BIN would turn that into an unbootable game. For a sane + // file this is a no-op, since the extent always fits in its sectors. + if (isFile) { + const u64 numBlocks = blockDevice->GetNumBlocks(); + const u64 firstSector = dir.firstDataSector; + const s64 availableBytes = firstSector >= numBlocks ? 0 : (s64)((numBlocks - firstSector) * (u64)sectorSize); + if (entry->size > availableBytes) { + entry->size = availableBytes; + } + } + if (entry->isDirectory && !relative) { if (entry->startsector == root->startsector) { blockDevice->NotifyReadError(); @@ -485,7 +499,11 @@ int ISOFileSystem::Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outd } VolDescriptor desc; - blockDevice->ReadBlock(16, (u8 *)&desc); + if (!blockDevice->ReadBlock(16, (u8 *)&desc)) { + blockDevice->NotifyReadError(); + ERROR_LOG(Log::FileSystem, "Failed to read volume descriptor for the path table"); + return SCE_KERNEL_ERROR_ERRNO_IO_ERROR; + } if (outlen < (u32)desc.pathTableLength) { return SCE_KERNEL_ERROR_ERRNO_INVALID_ARGUMENT; } else { @@ -503,7 +521,9 @@ int ISOFileSystem::Ioctl(u32 handle, u32 cmd, u32 indataPtr, u32 inlen, u32 outd u8 temp[2048]; // `blocks` whole sectors starting at `block` were already consumed by // ReadBlocks() above, so the trailing partial sector is the next one. - blockDevice->ReadBlock(block + blocks, temp); + if (!blockDevice->ReadBlock(block + blocks, temp)) { + memset(temp, 0, sizeof(temp)); + } memcpy(out, temp, size); } return 0; @@ -602,7 +622,11 @@ size_t ISOFileSystem::ReadFile(u32 handle, u8 *pointer, s64 size, int &usec) { const u8 *const start = pointer; if (firstBlockSize > 0) { - blockDevice->ReadBlock(secNum++, theSector); + // theSector is uninitialized stack memory, so on a failed read we must not copy it out - + // that would hand host stack contents to the game. + if (!blockDevice->ReadBlock(secNum++, theSector)) { + memset(theSector, 0, sizeof(theSector)); + } memcpy(pointer, theSector + firstBlockOffset, firstBlockSize); pointer += firstBlockSize; } @@ -613,7 +637,9 @@ size_t ISOFileSystem::ReadFile(u32 handle, u8 *pointer, s64 size, int &usec) { pointer += middleSize; } if (lastBlockSize > 0) { - blockDevice->ReadBlock(secNum++, theSector); + if (!blockDevice->ReadBlock(secNum++, theSector)) { + memset(theSector, 0, sizeof(theSector)); + } memcpy(pointer, theSector, lastBlockSize); pointer += lastBlockSize; } diff --git a/Core/Util/GameManager.cpp b/Core/Util/GameManager.cpp index 8387168892..ba4eb5be27 100644 --- a/Core/Util/GameManager.cpp +++ b/Core/Util/GameManager.cpp @@ -549,6 +549,7 @@ std::string GameManager::GetPBPGameID(FileLoader *loader) const { } std::string GameManager::GetISOGameID(FileLoader *loader) const { + static constexpr s64 MAX_PARAM_SFO_SIZE = 1024 * 1024; SequentialHandleAllocator handles; std::string errorString; std::shared_ptr bd(ConstructBlockDevice(loader, &errorString)); @@ -566,10 +567,16 @@ std::string GameManager::GetISOGameID(FileLoader *loader) const { if (handle < 0) { return ""; } + if (info.size < 0 || info.size > MAX_PARAM_SFO_SIZE) { + WARN_LOG(Log::Loader, "Ignoring implausibly large PARAM.SFO (%lld bytes)", (long long)info.size); + umd.CloseFile(handle); + return ""; + } std::string sfoData; - sfoData.resize(info.size); - umd.ReadFile(handle, (u8 *)&sfoData[0], info.size); + sfoData.resize((size_t)info.size); + if (info.size > 0) + umd.ReadFile(handle, (u8 *)sfoData.data(), info.size); umd.CloseFile(handle); ParamSFOData sfo; diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp index 881f8d550e..35a9662e35 100644 --- a/UI/GameInfoCache.cpp +++ b/UI/GameInfoCache.cpp @@ -450,11 +450,16 @@ void GameInfo::SetupTexture(Draw::DrawContext *thin3d, GameInfoTex &tex, int max // Will clear contents on failure. static bool ReadFileToString(IFileSystem *fs, std::string_view filename, std::string *contents, std::mutex *mtx) { + static constexpr s64 MAX_GAME_INFO_FILE_SIZE = 64 * 1024 * 1024; std::string fn(filename); PSPFileInfo info = fs->GetFileInfo(fn); if (!info.exists) { return false; } + if (info.size < 0 || info.size > MAX_GAME_INFO_FILE_SIZE) { + WARN_LOG(Log::UI, "Ignoring implausibly large game metadata file %s (%lld bytes)", fn.c_str(), (long long)info.size); + return false; + } int handle = fs->OpenFile(fn, FILEACCESS_READ); if (handle < 0) {