From 779cae623260bdff0cc5d70b468eb1255c97ea07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 11:47:55 +0200 Subject: [PATCH 1/8] Reject path traversal in savedata name fields gameName/saveName/fileName and the saveNameList entries are guest-controlled and get concatenated into host filesystem paths, so a crafted request could escape the save directory with ../ sequences. - PSPSaveDialog::Init rejects requests whose name fields contain a path separator ('/' or '\') or are bare dot components. - SavedataParam::SetPspParam rejects saveNameList entries the same way. --- Core/Dialog/PSPSaveDialog.cpp | 20 ++++++++++++++++++++ Core/Dialog/SavedataParam.cpp | 14 ++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/Core/Dialog/PSPSaveDialog.cpp b/Core/Dialog/PSPSaveDialog.cpp index 0e59fb80a9ab..6ac1da55e235 100755 --- a/Core/Dialog/PSPSaveDialog.cpp +++ b/Core/Dialog/PSPSaveDialog.cpp @@ -136,6 +136,26 @@ int PSPSaveDialog::Init(int paramAddr) { Memory::Memcpy(&request, requestAddr, size); Memory::Memcpy(&originalRequest, requestAddr, size); + // gameName/saveName/fileName become parts of host filesystem paths. + // Reject path separators (either direction) and bare dot components so a + // crafted request can't escape the save directory (path traversal). + auto hasPathTraversal = [](const char *field, size_t fieldSize) { + size_t len = 0; + while (len < fieldSize && field[len] != 0) + len++; + for (size_t i = 0; i < len; i++) { + if (field[i] == '/' || field[i] == '\\') + return true; + } + return (len == 1 && field[0] == '.') || (len == 2 && field[0] == '.' && field[1] == '.'); + }; + if (hasPathTraversal(request.gameName, sizeof(request.gameName)) || + hasPathTraversal(request.saveName, sizeof(request.saveName)) || + hasPathTraversal(request.fileName, sizeof(request.fileName))) { + ERROR_LOG_REPORT(Log::sceUtility, "sceUtilitySavedataInitStart: path separator in name fields"); + return SCE_ERROR_UTILITY_INVALID_PARAM_SIZE; + } + param.SetIgnoreTextures(IsNotVisibleAction((SceUtilitySavedataType)(u32)request.mode)); param.ClearSFOCache(); int retval = param.SetPspParam(&request); diff --git a/Core/Dialog/SavedataParam.cpp b/Core/Dialog/SavedataParam.cpp index 08d7dcb4c0d3..1a7610d40ca1 100644 --- a/Core/Dialog/SavedataParam.cpp +++ b/Core/Dialog/SavedataParam.cpp @@ -1547,6 +1547,20 @@ int SavedataParam::SetPspParam(SceUtilitySavedataParam *param) { // Get number of fileName in array saveDataListCount = 0; while (saveNameListData[saveDataListCount][0] != 0) { + // saveName entries become part of host filesystem paths; reject + // path separators and bare dot components (path traversal). + const std::string_view entry = StringViewFromFixedSizeField(saveNameListData[saveDataListCount]); + bool hasSeparator = false; + for (char c : entry) { + if (c == '/' || c == '\\') { + hasSeparator = true; + break; + } + } + if (hasSeparator || entry == "." || entry == "..") { + ERROR_LOG(Log::sceUtility, "SavedataParam: invalid saveName in list: %s", std::string(entry).c_str()); + return SCE_ERROR_UTILITY_INVALID_PARAM_SIZE; + } saveDataListCount++; } From 0eef5d0164e1e435e94a44d7536bb4f1b33da80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 11:51:10 +0200 Subject: [PATCH 2/8] Use shared HasPathTraversal utility for savedata name validation Replace the inline lambdas in the savedata dialog with the new HasPathTraversal() helper in Core/Util/PathUtil. --- Core/Dialog/PSPSaveDialog.cpp | 21 ++++++--------------- Core/Dialog/SavedataParam.cpp | 10 ++-------- Core/Util/PathUtil.cpp | 6 ++++++ Core/Util/PathUtil.h | 5 +++++ 4 files changed, 19 insertions(+), 23 deletions(-) diff --git a/Core/Dialog/PSPSaveDialog.cpp b/Core/Dialog/PSPSaveDialog.cpp index 6ac1da55e235..05700dfd8c5e 100755 --- a/Core/Dialog/PSPSaveDialog.cpp +++ b/Core/Dialog/PSPSaveDialog.cpp @@ -29,6 +29,7 @@ #include "Common/Thread/ThreadUtil.h" #include "Core/Dialog/PSPSaveDialog.h" #include "Core/FileSystems/MetaFileSystem.h" +#include "Core/Util/PathUtil.h" #include "Core/Util/PPGeDraw.h" #include "Common/TimeUtil.h" #include "Core/HLE/sceCtrl.h" @@ -137,21 +138,11 @@ int PSPSaveDialog::Init(int paramAddr) { Memory::Memcpy(&originalRequest, requestAddr, size); // gameName/saveName/fileName become parts of host filesystem paths. - // Reject path separators (either direction) and bare dot components so a - // crafted request can't escape the save directory (path traversal). - auto hasPathTraversal = [](const char *field, size_t fieldSize) { - size_t len = 0; - while (len < fieldSize && field[len] != 0) - len++; - for (size_t i = 0; i < len; i++) { - if (field[i] == '/' || field[i] == '\\') - return true; - } - return (len == 1 && field[0] == '.') || (len == 2 && field[0] == '.' && field[1] == '.'); - }; - if (hasPathTraversal(request.gameName, sizeof(request.gameName)) || - hasPathTraversal(request.saveName, sizeof(request.saveName)) || - hasPathTraversal(request.fileName, sizeof(request.fileName))) { + // Reject path separators and bare dot components so a crafted request + // can't escape the save directory (path traversal). + if (HasPathTraversal(StringViewFromFixedSizeField(request.gameName)) || + HasPathTraversal(StringViewFromFixedSizeField(request.saveName)) || + HasPathTraversal(StringViewFromFixedSizeField(request.fileName))) { ERROR_LOG_REPORT(Log::sceUtility, "sceUtilitySavedataInitStart: path separator in name fields"); return SCE_ERROR_UTILITY_INVALID_PARAM_SIZE; } diff --git a/Core/Dialog/SavedataParam.cpp b/Core/Dialog/SavedataParam.cpp index 1a7610d40ca1..fefa93de86c9 100644 --- a/Core/Dialog/SavedataParam.cpp +++ b/Core/Dialog/SavedataParam.cpp @@ -30,6 +30,7 @@ #include "Core/Dialog/SavedataParam.h" #include "Core/Dialog/PSPSaveDialog.h" #include "Core/FileSystems/MetaFileSystem.h" +#include "Core/Util/PathUtil.h" #include "Core/HLE/sceIo.h" #include "Core/HLE/sceKernelMemory.h" #include "Core/HLE/sceChnnlsv.h" @@ -1550,14 +1551,7 @@ int SavedataParam::SetPspParam(SceUtilitySavedataParam *param) { // saveName entries become part of host filesystem paths; reject // path separators and bare dot components (path traversal). const std::string_view entry = StringViewFromFixedSizeField(saveNameListData[saveDataListCount]); - bool hasSeparator = false; - for (char c : entry) { - if (c == '/' || c == '\\') { - hasSeparator = true; - break; - } - } - if (hasSeparator || entry == "." || entry == "..") { + if (HasPathTraversal(entry)) { ERROR_LOG(Log::sceUtility, "SavedataParam: invalid saveName in list: %s", std::string(entry).c_str()); return SCE_ERROR_UTILITY_INVALID_PARAM_SIZE; } diff --git a/Core/Util/PathUtil.cpp b/Core/Util/PathUtil.cpp index a5cb1a20d8d7..97404e9f1c10 100644 --- a/Core/Util/PathUtil.cpp +++ b/Core/Util/PathUtil.cpp @@ -24,6 +24,12 @@ bool HasParentDirComponent(std::string_view path) { return false; } +bool HasPathTraversal(std::string_view path) { + if (path.find_first_of("/\\") != std::string_view::npos) + return true; + return path == "." || path == ".."; +} + Path FindConfigFile(const Path &searchPath, std::string_view baseFilename, bool *exists) { // Don't search for an absolute path. if (baseFilename.size() > 1 && baseFilename[0] == '/') { diff --git a/Core/Util/PathUtil.h b/Core/Util/PathUtil.h index 0b7c81204475..c69ea2f7dbdc 100644 --- a/Core/Util/PathUtil.h +++ b/Core/Util/PathUtil.h @@ -34,6 +34,11 @@ enum PSPDirectories { // extracting or writing files to disk. bool HasParentDirComponent(std::string_view path); +// Returns true if the given string contains a path separator ('/' or '\\') +// or is a bare dot component ("." or ".."). Used to reject guest-controlled +// strings that would otherwise become host filesystem path components. +bool HasPathTraversal(std::string_view path); + Path FindConfigFile(const Path &searchPath, std::string_view baseFilename, bool *exists); Path GetSysDirectory(PSPDirectories directoryType); bool CreateSysDirectories(); From 6d43b6f531c0a42ce9b0a064c0e193ab0cf98a57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 11:53:21 +0200 Subject: [PATCH 3/8] Fix OOB read/write via negative seek wrap in VFSFileSystem SeekFile stored a signed s32 position into the unsigned size_t seekPos, so a negative position (e.g. from a truncating s32 cast of a large lseek offset) wrapped seekPos to near 2^64. ReadFile's clamp arithmetic then also wrapped, driving a memcpy from a wild pointer. - Clamp the computed seek position to 0 in SeekFile. - Clamp the read size against the remaining data in ReadFile, returning 0 when seekPos is at or past the end. --- Core/FileSystems/DirectoryFileSystem.cpp | 30 +++++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp index ed98a9b851b6..1350d4a3599c 100644 --- a/Core/FileSystems/DirectoryFileSystem.cpp +++ b/Core/FileSystems/DirectoryFileSystem.cpp @@ -1170,12 +1170,18 @@ size_t VFSFileSystem::ReadFile(u32 handle, u8 *pointer, s64 size, int &usec) { EntryMap::iterator iter = entries.find(handle); if (iter != entries.end()) { - if(iter->second.seekPos + size > iter->second.size) - size = iter->second.size - iter->second.seekPos; - if(size < 0) size = 0; - size_t bytesRead = size; - memcpy(pointer, iter->second.fileData + iter->second.seekPos, size); - iter->second.seekPos += size; + // Guard against a seekPos at or past the end of the file. Without + // this, a wrapped seekPos (near 2^64) makes the clamp arithmetic + // below wrap too, leading to an out-of-bounds memcpy. + if (iter->second.seekPos >= iter->second.size) + return 0; + + size_t bytesRead = (size_t)size; + size_t remaining = iter->second.size - iter->second.seekPos; + if (bytesRead > remaining) + bytesRead = remaining; + memcpy(pointer, iter->second.fileData + iter->second.seekPos, bytesRead); + iter->second.seekPos += bytesRead; return bytesRead; } else { ERROR_LOG(Log::FileSystem,"Cannot read file that hasn't been opened: %08x", handle); @@ -1196,11 +1202,17 @@ size_t VFSFileSystem::WriteFile(u32 handle, const u8 *pointer, s64 size, int &us size_t VFSFileSystem::SeekFile(u32 handle, s32 position, FileMove type) { EntryMap::iterator iter = entries.find(handle); if (iter != entries.end()) { + s64 newPos = 0; switch (type) { - case FILEMOVE_BEGIN: iter->second.seekPos = position; break; - case FILEMOVE_CURRENT: iter->second.seekPos += position; break; - case FILEMOVE_END: iter->second.seekPos = iter->second.size + position; break; + case FILEMOVE_BEGIN: newPos = position; break; + case FILEMOVE_CURRENT: newPos = (s64)iter->second.seekPos + position; break; + case FILEMOVE_END: newPos = (s64)iter->second.size + position; break; } + // Clamp to 0 so a negative position can't wrap the unsigned seekPos + // to a huge value (which would then read out of bounds). + if (newPos < 0) + newPos = 0; + iter->second.seekPos = (size_t)newPos; return iter->second.seekPos; } else { //This shouldn't happen... From 725cd691b3a0227fa2e9f0762e3def53e9373859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 12:07:00 +0200 Subject: [PATCH 4/8] Limit total extracted size when installing zips (zip bomb protection) ExtractZipContents wrote every entry's declared size with no ceiling, so a small high-ratio zip could decompress to fill the storage device. - Add a maxTotalSize parameter to ExtractZipContents (default 4GB) and ExtractFile. - Bail out in the size-summation pass when the total declared size exceeds the limit, and again per write chunk in case declared sizes are inaccurate. --- Core/Util/GameManager.cpp | 25 +++++++++++++++++++++---- Core/Util/GameManager.h | 5 +++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/Core/Util/GameManager.cpp b/Core/Util/GameManager.cpp index ff6fb234390c..9b877b0eb3a3 100644 --- a/Core/Util/GameManager.cpp +++ b/Core/Util/GameManager.cpp @@ -562,7 +562,7 @@ std::string GameManager::GetISOGameID(FileLoader *loader) const { return sfo.GetValueString("DISC_ID"); } -bool GameManager::ExtractFile(struct zip *z, int file_index, const Path &outFilename, size_t *bytesCopied, size_t allBytes) { +bool GameManager::ExtractFile(struct zip *z, int file_index, const Path &outFilename, size_t *bytesCopied, size_t allBytes, size_t maxTotalSize) { struct zip_stat zstat; zip_stat_index(z, file_index, 0, &zstat); size_t size = zstat.size; @@ -583,6 +583,16 @@ bool GameManager::ExtractFile(struct zip *z, int file_index, const Path &outFile u8 *buffer = new u8[blockSize]; while (pos < size) { size_t readSize = std::min(blockSize, size - pos); + // Stop before the total would exceed the limit (zip bomb), even + // if the declared sizes in the archive were inaccurate. + if (*bytesCopied > maxTotalSize || readSize > maxTotalSize - *bytesCopied) { + ERROR_LOG(Log::HLE, "Bailing: zip contents too large, limit %d", (int)maxTotalSize); + delete[] buffer; + fclose(f); + zip_fclose(zf); + File::Delete(outFilename); + return false; + } zip_int64_t retval = zip_fread(zf, buffer, readSize); if (retval < 0 || (size_t)retval < readSize) { ERROR_LOG(Log::HLE, "Failed to read %d bytes from zip (%d) - archive corrupt?", (int)readSize, (int)retval); @@ -625,7 +635,7 @@ bool GameManager::ExtractFile(struct zip *z, int file_index, const Path &outFile } // Doesn't care what it is, just extracts the whole ZIP to the requested location. -bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipFileInfo &info, bool allowRoot) { +bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipFileInfo &info, bool allowRoot, size_t maxTotalSize) { size_t allBytes = 0; size_t bytesCopied = 0; @@ -659,6 +669,7 @@ bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipF // Create all the directories first in one pass std::set createdDirs; + std::vector createdFiles; for (int i = 0; i < info.numFiles; i++) { // Let's count the directories as the first 10%. const char *fn = zip_get_name(z, i, 0); @@ -688,6 +699,13 @@ bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipF if (!isDir && fileAllowed(fn)) { struct zip_stat zstat; if (zip_stat_index(z, i, 0, &zstat) >= 0) { + // Guard against zip bombs: the total declared size must not + // exceed the limit. Check before adding to avoid overflow. + if (zstat.size > maxTotalSize || allBytes > maxTotalSize - zstat.size) { + ERROR_LOG(Log::HLE, "Bailing: zip contents too large (%d bytes), limit %d", (int)allBytes, (int)maxTotalSize); + SetInstallError(sy->T("Too large")); + goto bail; + } allBytes += zstat.size; } } @@ -697,7 +715,6 @@ bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipF INFO_LOG(Log::HLE, "Created %d directories", (int)createdDirs.size()); // Now, loop through again in a second pass, writing files. - std::vector createdFiles; for (int i = 0; i < info.numFiles; i++) { const char *fn = zip_get_name(z, i, 0); // Note that we do NOT write files that are not in a directory, to avoid random @@ -710,7 +727,7 @@ bool GameManager::ExtractZipContents(struct zip *z, const Path &dest, const ZipF if (isDir) continue; - if (!ExtractFile(z, i, outFilename, &bytesCopied, allBytes)) { + if (!ExtractFile(z, i, outFilename, &bytesCopied, allBytes, maxTotalSize)) { ERROR_LOG(Log::HLE, "Bailing: Failed to extract file: %s -> %s", zippedName.c_str(), outFilename.c_str()); goto bail; } else { diff --git a/Core/Util/GameManager.h b/Core/Util/GameManager.h index 9eb39f0bfdaf..9d75a450e1b0 100644 --- a/Core/Util/GameManager.h +++ b/Core/Util/GameManager.h @@ -89,7 +89,8 @@ class GameManager { bool UninstallGameOnThread(const std::string &name); // Extracts the contents of an open zip archive into dest. Exposed for testing. - bool ExtractZipContents(struct zip *z, const Path &dest, const ZipFileInfo &info, bool allowRoot); + // maxTotalSize limits the total decompressed size (zip bomb protection). + bool ExtractZipContents(struct zip *z, const Path &dest, const ZipFileInfo &info, bool allowRoot, size_t maxTotalSize = 0x100000000); private: void InstallZipContents(ZipFileTask task); @@ -99,7 +100,7 @@ class GameManager { void InstallDone(); - bool ExtractFile(struct zip *z, int file_index, const Path &outFilename, size_t *bytesCopied, size_t allBytes); + bool ExtractFile(struct zip *z, int file_index, const Path &outFilename, size_t *bytesCopied, size_t allBytes, size_t maxTotalSize = 0x100000000); bool DetectTexturePackDest(struct zip *z, int iniIndex, Path &dest); void SetInstallError(std::string_view err); From 942a76cfaa175331bf4052b4a1895b8ea975f36b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 12:11:21 +0200 Subject: [PATCH 5/8] Fix heap overflow via unsigned underflow in legacy Atrac::EnqueueForSas The space-left clamp was computed in unsigned 32-bit arithmetic, so a crafted fileoffset/FirstOffsetExtra could underflow to a huge value and leave addbytes unclamped, driving a Memory::Memcpy past dataBuf_. Compute the clamp in signed 64-bit, clamp negatives to 0, and skip the copy when there's nothing to write. --- Core/HLE/AtracCtx.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/Core/HLE/AtracCtx.cpp b/Core/HLE/AtracCtx.cpp index 1906b8070891..5c274d7fb7c4 100644 --- a/Core/HLE/AtracCtx.cpp +++ b/Core/HLE/AtracCtx.cpp @@ -960,15 +960,24 @@ void Atrac::CheckForSas() { } int Atrac::EnqueueForSas(u32 bufPtr, u32 bytesToAdd) { - int addbytes = std::min(bytesToAdd, track_.fileSize - first_.fileoffset - track_.FirstOffsetExtra()); - Memory::Memcpy(dataBuf_ + first_.fileoffset + track_.FirstOffsetExtra(), bufPtr, addbytes, "AtracAddStreamData"); + // Compute in signed 64-bit so an attacker-controlled fileoffset / + // FirstOffsetExtra can't underflow the space-left clamp and leave + // addbytes unclamped. + const s64 destOffset = (s64)first_.fileoffset + track_.FirstOffsetExtra(); + const s64 spaceLeft = (s64)track_.fileSize - destOffset; + s64 addbytes = std::min((s64)bytesToAdd, spaceLeft); + if (addbytes < 0) + addbytes = 0; + if (addbytes > 0) { + Memory::Memcpy(dataBuf_ + destOffset, bufPtr, (size_t)addbytes, "AtracAddStreamData"); + } first_.size += bytesToAdd; if (first_.size >= track_.fileSize) { first_.size = track_.fileSize; if (bufferState_ == ATRAC_STATUS_HALFWAY_BUFFER) bufferState_ = ATRAC_STATUS_ALL_DATA_LOADED; } - first_.fileoffset += addbytes; + first_.fileoffset += (u32)addbytes; // refresh context_ WriteContextToPSPMem(); return 0; From 77ebdf2e80ae90c0631fd7417f3cb55821b1ae1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 12:14:19 +0200 Subject: [PATCH 6/8] Clamp segment count in PSPModule::GetLongInfo nm.nsegment is attacker-controlled but segmentaddr/segmentsize are fixed 4-entry arrays; the debug info loop read past them. Clamp to 4 like the other consumers. --- Core/HLE/sceKernelModule.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index 714b3c48310e..4ef998c915fc 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -358,7 +358,9 @@ void PSPModule::GetLongInfo(char *ptr, int bufSize) const { StringWriter w(ptr, bufSize); w.F("%s: Version %d.%d. %d segments", nm.name, nm.version[1], nm.version[0], nm.nsegment).endl(); w.F("Memory block: %08x (%08x/%d bytes)", memoryBlockAddr, memoryBlockSize, memoryBlockSize).endl(); - for (int i = 0; i < (int)nm.nsegment; i++) { + // nm.nsegment is attacker-controlled (up to u32 max) but segmentaddr/ + // segmentsize are fixed 4-entry arrays; clamp like the other consumers. + for (int i = 0; i < (int)nm.nsegment && i < 4; i++) { w.F(" %08x (%08x bytes)\n", nm.segmentaddr[i], nm.segmentsize[i]); } w.F("Text: %08x (%08x bytes)\n", nm.text_addr, nm.text_size); From 5e6a051952b361178484cc250a00e2618a00b61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 12:18:08 +0200 Subject: [PATCH 7/8] Fix path traversal in texture pack filenames; use HasPathTraversal in savestate migration Texture pack filenames/aliases from textures.ini were used to build read and write paths with no '..' check, letting a malicious pack read or write files outside the pack directory. - LoadIniValues rejects entries with a parent dir component via HasParentDirComponent. - ReplacedTexture::Prepare skips such filenames as defense in depth. - PSPLoaders savestate migration now uses the shared HasPathTraversal helper instead of inline separator checks. --- Core/PSPLoaders.cpp | 6 ++---- GPU/Common/ReplacedTexture.cpp | 6 ++++++ GPU/Common/TextureReplacer.cpp | 7 +++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Core/PSPLoaders.cpp b/Core/PSPLoaders.cpp index 3be69e371b02..0deffb65bab1 100644 --- a/Core/PSPLoaders.cpp +++ b/Core/PSPLoaders.cpp @@ -429,10 +429,8 @@ bool Load_PSP_ELF_PBP(FileLoader *fileLoader, std::string_view discId, bool load // if any of them contain a path separator, skip the migration to avoid // building traversal paths. const bool anyPathSeparator = - discID.find('/') != std::string::npos || discID.find('\\') != std::string::npos || - discVersion.find('/') != std::string::npos || discVersion.find('\\') != std::string::npos || - homebrewName.find('/') != std::string::npos || homebrewName.find('\\') != std::string::npos || - madeUpID.find('/') != std::string::npos || madeUpID.find('\\') != std::string::npos; + HasPathTraversal(discID) || HasPathTraversal(discVersion) || + HasPathTraversal(homebrewName) || HasPathTraversal(madeUpID); const Path savestateDir = GetSysDirectory(DIRECTORY_SAVESTATE); for (int i = 0; i < 5 && !anyPathSeparator; ++i) { Path newPrefix = savestateDir / StringFromFormat("%s_%s_%d", discID.c_str(), discVersion.c_str(), i); diff --git a/GPU/Common/ReplacedTexture.cpp b/GPU/Common/ReplacedTexture.cpp index 6e1b2d68992e..ad306ecbae35 100644 --- a/GPU/Common/ReplacedTexture.cpp +++ b/GPU/Common/ReplacedTexture.cpp @@ -26,6 +26,7 @@ #include "GPU/Common/ReplacedTexture.h" #include "GPU/Common/TextureReplacer.h" +#include "Core/Util/PathUtil.h" #include "Common/Data/Format/DDSLoad.h" #include "Common/Data/Format/ZIMLoad.h" @@ -224,6 +225,11 @@ void ReplacedTexture::Prepare(VFSBackend *vfs) { } std::string path(desc_.filenames[i]); + // Defense in depth: skip filenames that could escape the pack dir. + if (HasParentDirComponent(path)) { + SetState(ReplacementState::CANCEL_INIT); + return; + } VFSFileReference *fileRef = vfs_->GetFile(path.c_str()); if (!fileRef) { if (i == 0) { diff --git a/GPU/Common/TextureReplacer.cpp b/GPU/Common/TextureReplacer.cpp index beac53b348a9..683156ffb97f 100644 --- a/GPU/Common/TextureReplacer.cpp +++ b/GPU/Common/TextureReplacer.cpp @@ -32,6 +32,7 @@ #include "Common/File/VFS/ZipFileReader.h" #include "Common/File/FileUtil.h" #include "Common/File/VFS/VFS.h" +#include "Core/Util/PathUtil.h" #include "Common/StringUtils.h" #include "Common/System/OSD.h" #include "Common/Thread/ThreadManager.h" @@ -358,6 +359,12 @@ bool TextureReplacer::LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverri truncate_cpy(k, line.Key()); std::string_view v = line.Value(); if (sscanf(k, "%16llx%8x_%d", &key.cachekey, &key.hash, &level) >= 1) { + // Reject path traversal: a "../" component could make us read + // or write files outside the texture pack directory. + if (HasParentDirComponent(v)) { + ERROR_LOG(Log::TexReplacement, "Ignoring texture filename with parent dir component: %s", std::string(v).c_str()); + continue; + } // We allow empty filenames, to mark textures that we don't want to keep saving. filenameMap[key][level] = v; if (checkFilenames) { From 0049b19fbf8853066c96fc0ce81652fcda1f5075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 12:43:10 +0200 Subject: [PATCH 8/8] Fix OOB in texture replacer mip mixing and non-DXT texture decode Vuln 17: ReplacedTexture::LoadLevelData let a KTX2/DDS file at a higher mip level resize the shared data_ vector to its own (attacker-controlled) mip count, so data_[mipLevel + i] indexed out of bounds and the KTX2 branch resized a different element than it wrote to. Disallow mixing image formats across mip levels, cap the container mip count, and resize the same element that is used as the transcode destination. Vuln 18: DecodeTextureLevel only validated the start address for non-DXT textures, so guest-controlled w/h/bufw could drive reads past mapped RAM. Validate the needed range like the DXT path does and clamp the height; ReadIndexedTex now takes the clamped w/h. --- GPU/Common/ReplacedTexture.cpp | 29 ++++++++++++++++++++++++++--- GPU/Common/ReplacedTexture.h | 3 +++ GPU/Common/TextureCacheCommon.cpp | 26 +++++++++++++++++++------- GPU/Common/TextureCacheCommon.h | 2 +- 4 files changed, 49 insertions(+), 11 deletions(-) diff --git a/GPU/Common/ReplacedTexture.cpp b/GPU/Common/ReplacedTexture.cpp index ad306ecbae35..3590d79bd2b7 100644 --- a/GPU/Common/ReplacedTexture.cpp +++ b/GPU/Common/ReplacedTexture.cpp @@ -321,6 +321,17 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference std::string magic; ReplacedImageType imageType = Identify(vfs_, openFile, &magic); + // Disallow mixing image formats across mip levels: a KTX2/DDS container + // manages its own mip chain, so mixing one in at a higher mip level + // would corrupt the shared level data layout. + if (mipLevel == 0) { + firstImageType_ = imageType; + } else if (imageType != firstImageType_) { + WARN_LOG(Log::TexReplacement, "Replacement mipmap %d uses image format %d, but mip 0 uses %d. Stopping.", mipLevel, (int)imageType, (int)firstImageType_); + vfs_->CloseFile(openFile); + return LoadLevelResult::DONE; + } + bool ddsDX10 = false; int numMips = 1; @@ -531,7 +542,13 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference WARN_LOG(Log::TexReplacement, "Block compressed replacement texture '%s' not divisible by 4x4 (%dx%d). In D3D11 (only!) we will have to expand (potentially causing glitches).", filename.c_str(), level.w, level.h); } - data_.resize(numMips); + // Cap the mip count (attacker-controlled header field) and make sure + // data_ is large enough for mipLevel + numMips; otherwise the loop + // below indexes past the end of data_. + numMips = std::max(1, std::min(numMips, MAX_REPLACEMENT_MIP_LEVELS - mipLevel)); + if ((size_t)(mipLevel + numMips) > data_.size()) { + data_.resize(mipLevel + numMips); + } basist::ktx2_transcoder_state transcodeState; // Each thread needs one of these. @@ -553,7 +570,7 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference outputSize = levelInfo.m_orig_width * levelInfo.m_orig_height; outputPitch = levelInfo.m_orig_width; } - data_[i].resize(dataSizeBytes); + out.resize(dataSizeBytes); transcodeState.clear(); transcoder.transcode_image_level(i, 0, 0, &out[0], (uint32_t)outputSize, transcoderFormat, 0, (uint32_t)outputPitch, level.h, -1, -1, &transcodeState); @@ -585,7 +602,13 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference WARN_LOG(Log::TexReplacement, "Block compressed replacement texture '%s' not divisible by 4x4 (%dx%d). In D3D11 (only!) we will have to expand (potentially causing glitches).", filename.c_str(), level.w, level.h); } - data_.resize(numMips); + // Cap the mip count (attacker-controlled header field) and make sure + // data_ is large enough for mipLevel + numMips; otherwise the loop + // below indexes past the end of data_. + numMips = std::max(1, std::min(numMips, MAX_REPLACEMENT_MIP_LEVELS - mipLevel)); + if ((size_t)(mipLevel + numMips) > data_.size()) { + data_.resize(mipLevel + numMips); + } // A DDS File can contain multiple mipmaps. levels_.reserve(numMips); diff --git a/GPU/Common/ReplacedTexture.h b/GPU/Common/ReplacedTexture.h index 540c3574d598..c8496111041b 100644 --- a/GPU/Common/ReplacedTexture.h +++ b/GPU/Common/ReplacedTexture.h @@ -220,6 +220,9 @@ class ReplacedTexture { std::vector> data_; std::vector levels_; + // Image format of mip level 0; mixing formats across levels is not + // allowed (container formats like KTX2/DDS manage their own mip chain). + ReplacedImageType firstImageType_ = ReplacedImageType::INVALID; double lastUsed_ = 0.0; LimitedWaitable *threadWaitable_ = nullptr; diff --git a/GPU/Common/TextureCacheCommon.cpp b/GPU/Common/TextureCacheCommon.cpp index 25ece3e25b4d..f6c7df252ed6 100644 --- a/GPU/Common/TextureCacheCommon.cpp +++ b/GPU/Common/TextureCacheCommon.cpp @@ -1957,6 +1957,21 @@ TextureAlpha TextureCacheCommon::DecodeTextureLevel(u8 *out, int outPitch, GETex const u8 *texptr = Memory::GetPointer(texaddr); const uint32_t byteSize = (textureBitsPerPixel[format] * bufw * h) / 8; + // Validate the texture data fits in mapped RAM, like the DXT path does. + // texaddr/bufw/w/h are all guest-controlled via the GE display list. + const int bpp = textureBitsPerPixel[format]; + const uint32_t bytesPerRow = (bpp * bufw) / 8; + // Swizzled textures are read in 8-row blocks, rounding the height up. + const uint32_t rows = swizzled ? ((h + 7) & ~7) : h; + const uint32_t neededBytes = bytesPerRow * rows; + if (bytesPerRow > 0 && !Memory::IsValidRange(texaddr, neededBytes)) { + ERROR_LOG_REPORT(Log::G3D, "Texture extends beyond valid RAM: %08x + %d x %d", texaddr, bufw, h); + uint32_t limited = Memory::ClampValidSizeAt(texaddr, neededBytes); + h = limited / bytesPerRow; + if (swizzled) + h &= ~7; + } + char buf[128]; size_t len = snprintf(buf, sizeof(buf), "Tex_%08x_%dx%d_%s", texaddr, w, h, GeTextureFormatToString(format, clutformat)); NotifyMemInfo(MemBlockFlags::TEXTURE, texaddr, byteSize, buf, len); @@ -2065,13 +2080,13 @@ TextureAlpha TextureCacheCommon::DecodeTextureLevel(u8 *out, int outPitch, GETex // We can't know anything about alpha. return TextureAlpha::Any; } - return ReadIndexedTex(out, outPitch, level, texptr, 1, bufw, reverseColors, expandTo32bit); + return ReadIndexedTex(out, outPitch, w, h, level, texptr, 1, bufw, reverseColors, expandTo32bit); case GE_TFMT_CLUT16: - return ReadIndexedTex(out, outPitch, level, texptr, 2, bufw, reverseColors, expandTo32bit); + return ReadIndexedTex(out, outPitch, w, h, level, texptr, 2, bufw, reverseColors, expandTo32bit); case GE_TFMT_CLUT32: - return ReadIndexedTex(out, outPitch, level, texptr, 4, bufw, reverseColors, expandTo32bit); + return ReadIndexedTex(out, outPitch, w, h, level, texptr, 4, bufw, reverseColors, expandTo32bit); case GE_TFMT_4444: case GE_TFMT_5551: @@ -2189,10 +2204,7 @@ TextureAlpha TextureCacheCommon::DecodeTextureLevel(u8 *out, int outPitch, GETex return AlphaSumIsFull(alphaSum, fullAlphaMask) ? TextureAlpha::Solid : TextureAlpha::Any; } -TextureAlpha TextureCacheCommon::ReadIndexedTex(u8 *out, int outPitch, int level, const u8 *texptr, int bytesPerIndex, int bufw, bool reverseColors, bool expandTo32Bit) { - int w = gstate.getTextureWidth(level); - int h = gstate.getTextureHeight(level); - +TextureAlpha TextureCacheCommon::ReadIndexedTex(u8 *out, int outPitch, int w, int h, int level, const u8 *texptr, int bytesPerIndex, int bufw, bool reverseColors, bool expandTo32Bit) { if (gstate.isTextureSwizzled()) { tmpTexBuf32_.resize(bufw * ((h + 7) & ~7)); UnswizzleFromMem(tmpTexBuf32_.data(), bufw * bytesPerIndex, texptr, bufw, h, bytesPerIndex); diff --git a/GPU/Common/TextureCacheCommon.h b/GPU/Common/TextureCacheCommon.h index f2f3b495234e..1908002ad764 100644 --- a/GPU/Common/TextureCacheCommon.h +++ b/GPU/Common/TextureCacheCommon.h @@ -425,7 +425,7 @@ class TextureCacheCommon { TextureAlpha DecodeTextureLevel(u8 *out, int outPitch, GETextureFormat format, GEPaletteFormat clutformat, uint32_t texaddr, int level, int bufw, TexDecodeFlags flags); static void UnswizzleFromMem(u32 *dest, u32 destPitch, const u8 *texptr, u32 bufw, u32 height, u32 bytesPerPixel); - TextureAlpha ReadIndexedTex(u8 *out, int outPitch, int level, const u8 *texptr, int bytesPerIndex, int bufw, bool reverseColors, bool expandTo32Bit); + TextureAlpha ReadIndexedTex(u8 *out, int outPitch, int w, int h, int level, const u8 *texptr, int bytesPerIndex, int bufw, bool reverseColors, bool expandTo32Bit); ReplacedTexture *FindReplacement(TexCacheEntry *entry, int *w, int *h, int *d); void PollReplacement(TexCacheEntry *entry, int *w, int *h, int *d);