Vulnerability fixes (3/3)

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"
@@ -136,6 +137,16 @@ 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 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;
+	}
+
 	param.SetIgnoreTextures(IsNotVisibleAction((SceUtilitySavedataType)(u32)request.mode));
 	param.ClearSFOCache();
 	int retval = param.SetPspParam(&request);

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"
@@ -1547,6 +1548,13 @@ 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]);
+			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;
+			}
 			saveDataListCount++;
 		}
 

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...

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>((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;

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);

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);

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<Path> createdDirs;
+	std::vector<Path> 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<Path> 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 {

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);
 

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] == '/') {

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();

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) {
@@ -315,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;
 
@@ -525,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.
 
@@ -547,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);
@@ -579,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);

GPU/Common/ReplacedTexture.h

@@ -220,6 +220,9 @@ class ReplacedTexture {
 
 	std::vector<std::vector<uint8_t>> data_;
 	std::vector<ReplacedTextureLevel> 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;