From b1ffcde9fcd478acd76b5d341390db98065b20ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 12:32:03 +0100 Subject: [PATCH 1/6] Audio, PSMF: Check the whole range the decoders touch - Atrac: the partial-frame buffer was sized from the codec field in the guest's context, which a game can change after SetData, so an Atrac3+ decoder could write 2048 samples into a 1024-sample buffer. Size it for any frame, and decide from the decoder whether a frame fits the output. Check the output buffer holds a frame and the input holds the frame read, not just their first bytes, and range-check the streaming buffer copy. - Atrac track parsing is bounded by mapped memory, not only the game's size. - sceMp3LowLevelDecode and sceAudiocodecDecode check their output holds a frame, and the MP3 input is clamped. FindMp3Header read 3 bytes past its range. - PSMF: the EP map address could wrap around to a valid one while the entries were read from the unwrapped host pointer, and scePsmfSetPsmf checked one byte of the 0x82-byte header it reads. Co-Authored-By: Claude Opus 5.5 --- Core/HLE/AtracCtx2.cpp | 25 ++++++++++++++++--------- Core/HLE/sceAtrac.cpp | 30 +++++++++++++++++++----------- Core/HLE/sceAudiocodec.cpp | 6 ++++++ Core/HLE/sceMp3.cpp | 12 +++++++++--- Core/HLE/scePsmf.cpp | 13 ++++++++++--- 5 files changed, 60 insertions(+), 26 deletions(-) diff --git a/Core/HLE/AtracCtx2.cpp b/Core/HLE/AtracCtx2.cpp index 32f353971c..afa9ea9ef2 100644 --- a/Core/HLE/AtracCtx2.cpp +++ b/Core/HLE/AtracCtx2.cpp @@ -875,9 +875,13 @@ u32 Atrac2::DecodeInternal(u32 outbufAddr, int *SamplesNum, int *finish) { int16_t *outPtr; _dbg_assert_(samplesToDecode <= info.SamplesPerFrame()); - if (samplesToDecode != info.SamplesPerFrame()) { + // info.codec is in guest memory and can be changed after SetData, so what the decoder actually + // writes is decided by the decoder. + const int decoderSamples = decoder_->GetAudioType() == PSP_CODEC_AT3PLUS ? ATRAC3PLUS_MAX_SAMPLES : ATRAC3_MAX_SAMPLES; + if (samplesToDecode != decoderSamples) { if (!decodeTemp_) { - decodeTemp_ = new int16_t[info.SamplesPerFrame() * outputChannels_]; + // Big enough for any frame, in any channel count. + decodeTemp_ = new int16_t[ATRAC3PLUS_MAX_SAMPLES * 2](); } outPtr = decodeTemp_; } else { @@ -887,8 +891,9 @@ u32 Atrac2::DecodeInternal(u32 outbufAddr, int *SamplesNum, int *finish) { context_->codec.inBuf = inAddr; context_->codec.outBuf = outbufAddr; - if (!Memory::IsValidAddress(inAddr)) { - ERROR_LOG(Log::Atrac, "DecodeInternal: Bad inAddr %08x", inAddr); + // The decoder reads a whole frame. + if (!Memory::IsValidRange(inAddr, info.sampleSize)) { + ERROR_LOG(Log::Atrac, "DecodeInternal: Bad inAddr %08x (frame size %d)", inAddr, info.sampleSize); return SCE_ERROR_ATRAC_API_FAIL; } @@ -918,8 +923,8 @@ u32 Atrac2::DecodeInternal(u32 outbufAddr, int *SamplesNum, int *finish) { *finish = 0; } u8 *outBuf = outbufAddr ? Memory::GetPointerWriteOrException(outbufAddr) : nullptr; - if (samplesToDecode != info.SamplesPerFrame() && samplesToDecode != 0 && outBuf) { - memcpy(outBuf, decodeTemp_, samplesToDecode * outputChannels_ * sizeof(int16_t)); + if (outPtr == decodeTemp_ && samplesToDecode != 0 && outBuf) { + memcpy(outBuf, decodeTemp_, std::min(samplesToDecode, (int)ATRAC3PLUS_MAX_SAMPLES) * outputChannels_ * sizeof(int16_t)); } // Handle increments and looping. @@ -976,9 +981,11 @@ u32 Atrac2::DecodeInternal(u32 outbufAddr, int *SamplesNum, int *finish) { size_t copyLen = info.secondBufferByte % info.sampleSize; if (copyLen > info.bufferByte) copyLen = info.bufferByte; - memcpy(Memory::GetPointerWriteOrException(info.buffer), - Memory::GetPointerOrException(info.secondBuffer + (info.secondBufferByte - info.secondBufferByte % info.sampleSize)), - copyLen); + const u32 copySrc = info.secondBuffer + (info.secondBufferByte - info.secondBufferByte % info.sampleSize); + // Both addresses come from the context in guest memory. + if (Memory::IsValidRange(info.buffer, (u32)copyLen) && Memory::IsValidRange(copySrc, (u32)copyLen)) { + memcpy(Memory::GetPointerWriteOrException(info.buffer), Memory::GetPointerOrException(copySrc), copyLen); + } } } } diff --git a/Core/HLE/sceAtrac.cpp b/Core/HLE/sceAtrac.cpp index dd414c63a8..61b9dd26c4 100644 --- a/Core/HLE/sceAtrac.cpp +++ b/Core/HLE/sceAtrac.cpp @@ -420,7 +420,8 @@ static u32 sceAtracDecodeData(int atracID, u32 outAddr, u32 numSamplesAddr, u32 int numSamplesWritten = 0; int finish = 0; int remains = 0; - if (outAddr != 0 && !Memory::IsValidAddress(outAddr)) { + // A whole frame is written. + if (outAddr != 0 && !Memory::IsValidRange(outAddr, atrac->GetOutputChannels() * 2 * atrac->SamplesPerFrame())) { // Dunno what error code to return here, but we have to bail in this case to avoid crashing PPSSPP. return hleLogError(Log::Atrac, SCE_ERROR_ATRAC_SIZE_TOO_SMALL); } @@ -757,6 +758,13 @@ static u32 sceAtracResetPlayPosition(int atracID, int sample, int bytesWrittenFi return hleDelayResult(res, "reset play pos", 3000); } + +// The track parsers read up to the size they're given, which the game picks. Bound it by what's +// actually mapped, but keep it for invalid pointers so those are still reported as such. +static u32 GuestReadableSize(u32 addr, u32 size) { + return Memory::IsValidAddress(addr) ? Memory::ClampValidSizeAt(addr, size) : size; +} + static u32 sceAtracSetHalfwayBuffer(int atracID, u32 buffer, u32 readSize, u32 bufferSize) { AtracBase *atrac = getAtrac(atracID); // Don't use AtracValidateManaged here. @@ -770,7 +778,7 @@ static u32 sceAtracSetHalfwayBuffer(int atracID, u32 buffer, u32 readSize, u32 b Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), readSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, readSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -807,7 +815,7 @@ static u32 sceAtracSetData(int atracID, u32 buffer, u32 bufferSize) { Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), bufferSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, bufferSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -838,7 +846,7 @@ static int sceAtracSetDataAndGetID(u32 buffer, int bufferSize) { std::string error; // We let zero-pointer through as zeroes, so the small size check can kick in before it's accessed. // This is needed in Tomb Raider Legend (#20145). - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), bufferSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, bufferSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -864,7 +872,7 @@ static int sceAtracSetHalfwayBufferAndGetID(u32 buffer, u32 readSize, u32 buffer Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), readSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, readSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -990,7 +998,7 @@ static int sceAtracSetMOutHalfwayBuffer(int atracID, u32 buffer, u32 readSize, u Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), readSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, readSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -1016,7 +1024,7 @@ static u32 sceAtracSetMOutData(int atracID, u32 buffer, u32 bufferSize) { Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), bufferSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, bufferSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -1036,7 +1044,7 @@ static u32 sceAtracSetMOutData(int atracID, u32 buffer, u32 bufferSize) { static int sceAtracSetMOutDataAndGetID(u32 buffer, u32 bufferSize) { Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), bufferSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, bufferSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -1065,7 +1073,7 @@ static int sceAtracSetMOutHalfwayBufferAndGetID(u32 buffer, u32 readSize, u32 bu Track track; std::string error; - int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), readSize, &track, &error); + int ret = AnalyzeAtracTrack(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, readSize), &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -1090,7 +1098,7 @@ static int sceAtracSetMOutHalfwayBufferAndGetID(u32 buffer, u32 readSize, u32 bu static int sceAtracSetAA3DataAndGetID(u32 buffer, u32 bufferSize, u32 fileSize, u32 metadataSizeAddr) { Track track; std::string error; - int ret = AnalyzeAA3Track(Memory::GetPointerOrNull(buffer), bufferSize, fileSize, &track, &error); + int ret = AnalyzeAA3Track(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, bufferSize), fileSize, &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } @@ -1116,7 +1124,7 @@ static int sceAtracSetAA3HalfwayBufferAndGetID(u32 buffer, u32 readSize, u32 buf Track track; std::string error; - int ret = AnalyzeAA3Track(Memory::GetPointerOrNull(buffer), readSize, fileSize, &track, &error); + int ret = AnalyzeAA3Track(Memory::GetPointerOrNull(buffer), GuestReadableSize(buffer, readSize), fileSize, &track, &error); if (ret < 0) { return hleLogError(Log::Atrac, ret, "%s", error.c_str()); } diff --git a/Core/HLE/sceAudiocodec.cpp b/Core/HLE/sceAudiocodec.cpp index c8f346cc3c..5e5514ab7e 100644 --- a/Core/HLE/sceAudiocodec.cpp +++ b/Core/HLE/sceAudiocodec.cpp @@ -640,6 +640,12 @@ static int sceAudiocodecDecode(u32 ctxPtr, int codec) { } else if (codec == PSP_CODEC_AT3PLUS && ctx->fmt.at3.outputChannels == 1) { outputChannels = 1; } + // The decoder writes a whole frame, but only the first byte of outBuf was checked. + const u32 maxFrameSamples = codec == PSP_CODEC_AT3PLUS ? 2048 : (codec == PSP_CODEC_MP3 ? 1152 : 1024); + if (!Memory::IsValidRange(ctx->outBuf, maxFrameSamples * outputChannels * 2)) { + ctx->err = 0x20b; + return hleLogError(Log::ME, 0, "Output buffer at %08x can't hold a frame", ctx->outBuf); + } bool result = decoder->Decode(inBuf, bytesPerFrame, &inDataConsumed, outputChannels, outBuf, &outSamples); if (!result && (codec == PSP_CODEC_AT3PLUS || codec == PSP_CODEC_AT3)) { // What the hardware reports for a frame that doesn't decode (0x208 is also possible for diff --git a/Core/HLE/sceMp3.cpp b/Core/HLE/sceMp3.cpp index acd4e9edec..b180b0bd11 100644 --- a/Core/HLE/sceMp3.cpp +++ b/Core/HLE/sceMp3.cpp @@ -420,9 +420,10 @@ static int CalculateMp3SamplesPerFrame(int versionBits, int layerBits) { static int FindMp3Header(AuCtx *ctx, int &header, int end) { u32 addr = ctx->AuBuf + ctx->AuStreamWorkareaSize(); - if (Memory::IsValidRange(addr, end)) { + // A header is four bytes, read at each offset. + if (end >= 4 && Memory::IsValidRange(addr, end)) { const u8 *ptr = Memory::GetPointerUnchecked(addr); - for (int offset = 0; offset < end; ++offset) { + for (int offset = 0; offset + 4 <= end; ++offset) { // If we hit valid sync bits, then we've found a header. if (ptr[offset] == 0xFF && (ptr[offset + 1] & 0xC0) == 0xC0) { header = bswap32(Memory::ReadUnchecked_U32(addr + offset)); @@ -758,12 +759,17 @@ static u32 sceMp3LowLevelDecode(u32 mp3, u32 sourceAddr, u32 sourceBytesConsumed return hleLogError(Log::ME, -1, "invalid address in args"); } + // A frame is at most 1152 stereo samples, and the decoder is handed up to 4096 bytes. + if (!Memory::IsValidRange(samplesAddr, 1152 * 2 * 2) || !Memory::IsValidRange(sourceBytesConsumedAddr, 4) || !Memory::IsValidRange(sampleBytesAddr, 4)) { + return hleLogError(Log::ME, -1, "invalid address in args"); + } const u8 *inbuff = Memory::GetPointerWriteUnchecked(sourceAddr); int16_t *outbuf = (int16_t *)Memory::GetPointerWriteUnchecked(samplesAddr); + const int inBytes = (int)Memory::ClampValidSizeAt(sourceAddr, 4096); int outSamples = 0; int inbytesConsumed = 0; - if (!ctx->decoder->Decode(inbuff, 4096, &inbytesConsumed, 2, outbuf, &outSamples)) { + if (!ctx->decoder->Decode(inbuff, inBytes, &inbytesConsumed, 2, outbuf, &outSamples)) { WARN_LOG(Log::ME, "sceMp3LowLevelDecode: Decode failed"); } int outBytes = outSamples * sizeof(int16_t) * 2; diff --git a/Core/HLE/scePsmf.cpp b/Core/HLE/scePsmf.cpp index f0c8070d27..d234f8ec41 100644 --- a/Core/HLE/scePsmf.cpp +++ b/Core/HLE/scePsmf.cpp @@ -295,9 +295,15 @@ public: // the unwrapped count. const u64 epMapBytes = EP_MAP_STRIDE * (u64)psmf->EPMapEntriesNum; if (psmf->headerOffset != 0) { - if (epMapBytes > 0xFFFFFFFFull - psmf->EPMapOffset || !Memory::IsValidRange(psmf->headerOffset, psmf->EPMapOffset + (u32)epMapBytes)) { + // The entries are read from data + EPMapOffset, so the map has to start inside the + // address space (the sum used to wrap around to a valid address) and be mapped. + const u64 epMapAddr = (u64)psmf->headerOffset + psmf->EPMapOffset; + if (epMapAddr > 0xFFFFFFFFull || !Memory::IsValidAddress((u32)epMapAddr)) { + ERROR_LOG(Log::ME, "Invalid PSMF EP map offset: %08x", psmf->EPMapOffset); + psmf->EPMapEntriesNum = 0; + } else if (epMapBytes > 0xFFFFFFFFull - psmf->EPMapOffset || !Memory::IsValidRange(psmf->headerOffset, psmf->EPMapOffset + (u32)epMapBytes)) { ERROR_LOG(Log::ME, "Invalid PSMF EP map entry count: %d", psmf->EPMapEntriesNum); - psmf->EPMapEntriesNum = Memory::ClampValidSizeAt(psmf->headerOffset + psmf->EPMapOffset, (u32)epMapBytes) / EP_MAP_STRIDE; + psmf->EPMapEntriesNum = Memory::ClampValidSizeAt((u32)epMapAddr, (u32)epMapBytes) / EP_MAP_STRIDE; } } else { // No guest address to validate against (player tempbuf path, @@ -770,7 +776,8 @@ static void DelayPsmfStateChange(u32 psmfPlayer, u32 newState, s64 delayUs) { } static u32 scePsmfSetPsmf(u32 psmfStruct, u32 psmfData) { - if (!Memory::IsValidAddress(psmfStruct) || !Memory::IsValidAddress(psmfData)) { + // The header fields Psmf reads go up to 0x82. + if (!Memory::IsValidAddress(psmfStruct) || !Memory::IsValidRange(psmfData, 0x82)) { // Crashes on a PSP. return hleReportError(Log::ME, SCE_KERNEL_ERROR_ILLEGAL_ADDRESS, "bad address"); } From 1021767568780037841e380df515da33adf9686b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 14:56:18 +0100 Subject: [PATCH 2/6] HLE utility libraries: Range-check buffers and contexts These checked the first byte of a buffer the game sized, then read or wrote all of it on the host: - sceDeflt: the output length went straight to zlib. - sceSfmt19937: array end checks wrapped in 32 bits (FillArray32 with a huge count wrote gigabytes), and the 2.5 KB state struct was only checked at its start. The same for the MT19937 contexts in sceMt19937 and sceKernelUtilsMt19937*. - sceChnnlsv: sceSdRemoveValue took any length (a negative one passed the key length check), sceSdSetMember decrypted alignedLen bytes in place, and the 16-byte hash, key and data buffers were checked for one byte. - MD5, SHA-1 and SHA-256 digests: check the data length and digest size (SHA-256 checked data + len, which wraps). - sceCcc: clamp the destination size to memory, and require the source to be terminated in mapped memory, since the converters walk it to its end. - sceRtc: the RFC formatters write 32 bytes, the Win32 file time functions a whole date struct, and the date parser needs a terminated string. Co-Authored-By: Claude Opus 5.5 --- Core/HLE/sceCcc.cpp | 39 +++++++++++++++++++++++++++------------ Core/HLE/sceChnnlsv.cpp | 10 +++++++++- Core/HLE/sceDeflt.cpp | 5 ++++- Core/HLE/sceMd5.cpp | 10 +++++----- Core/HLE/sceMt19937.cpp | 4 ++-- Core/HLE/sceRtc.cpp | 16 ++++++++-------- Core/HLE/sceSfmt19937.cpp | 22 ++++++++++++++++------ Core/HLE/sceSha256.cpp | 2 +- 8 files changed, 72 insertions(+), 36 deletions(-) diff --git a/Core/HLE/sceCcc.cpp b/Core/HLE/sceCcc.cpp index b8cf497a03..89d671b9ad 100644 --- a/Core/HLE/sceCcc.cpp +++ b/Core/HLE/sceCcc.cpp @@ -33,6 +33,21 @@ typedef PSPPointer PSPWCharPointer; typedef PSPPointer PSPConstCharPointer; typedef PSPPointer PSPConstWCharPointer; +// The converters walk the source to its terminator, so it has to have one in mapped memory. +static bool SourceTerminated(const PSPConstCharPointer &src) { + return Memory::IsValidNullTerminatedString(src.ptr); +} + +static bool SourceTerminated(const PSPConstWCharPointer &src) { + const u32 maxChars = Memory::MaxSizeAtAddress(src.ptr) / 2; + const char16_t *p = (const char16_t *)Memory::GetPointerUnchecked(src.ptr); + for (u32 i = 0; i < maxChars; i++) { + if (p[i] == 0) + return true; + } + return false; +} + static u16 errorUTF8; static u16 errorUTF16; static u16 errorSJIS; @@ -95,14 +110,14 @@ static int sceCccUTF8toUTF16(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstCharPointer::Create(srcAddr); auto dst = PSPWCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccUTF8toUTF16(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; } // Round dstSize down if it represents half a character. - const auto dstEnd = PSPWCharPointer::Create(dstAddr + (dstSize & ~1)); + const auto dstEnd = PSPWCharPointer::Create(dstAddr + (Memory::ClampValidSizeAt(dstAddr, dstSize) & ~1)); DEBUG_LOG(Log::sceMisc, "sceCccUTF8toUTF16(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); UTF8 utf(src); @@ -130,7 +145,7 @@ static int sceCccUTF8toSJIS(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstCharPointer::Create(srcAddr); auto dst = PSPCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccUTF8toSJIS(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; @@ -141,7 +156,7 @@ static int sceCccUTF8toSJIS(u32 dstAddr, u32 dstSize, u32 srcAddr) return 0; } - const auto dstEnd = PSPCharPointer::Create(dstAddr + dstSize); + const auto dstEnd = PSPCharPointer::Create(dstAddr + Memory::ClampValidSizeAt(dstAddr, dstSize)); DEBUG_LOG(Log::sceMisc, "sceCccUTF8toSJIS(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); UTF8 utf(src); @@ -169,13 +184,13 @@ static int sceCccUTF16toUTF8(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstWCharPointer::Create(srcAddr); auto dst = PSPCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccUTF16toUTF8(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; } - const auto dstEnd = PSPCharPointer::Create(dstAddr + dstSize); + const auto dstEnd = PSPCharPointer::Create(dstAddr + Memory::ClampValidSizeAt(dstAddr, dstSize)); DEBUG_LOG(Log::sceMisc, "sceCccUTF16toUTF8(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); UTF16LE utf(src); @@ -202,7 +217,7 @@ static int sceCccUTF16toSJIS(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstWCharPointer::Create(srcAddr); auto dst = PSPCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccUTF16toSJIS(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; @@ -213,7 +228,7 @@ static int sceCccUTF16toSJIS(u32 dstAddr, u32 dstSize, u32 srcAddr) return 0; } - const auto dstEnd = PSPCharPointer::Create(dstAddr + dstSize); + const auto dstEnd = PSPCharPointer::Create(dstAddr + Memory::ClampValidSizeAt(dstAddr, dstSize)); DEBUG_LOG(Log::sceMisc, "sceCccUTF16toSJIS(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); UTF16LE utf(src); @@ -240,7 +255,7 @@ static int sceCccSJIStoUTF8(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstCharPointer::Create(srcAddr); auto dst = PSPCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccSJIStoUTF8(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; @@ -251,7 +266,7 @@ static int sceCccSJIStoUTF8(u32 dstAddr, u32 dstSize, u32 srcAddr) return 0; } - const auto dstEnd = PSPCharPointer::Create(dstAddr + dstSize); + const auto dstEnd = PSPCharPointer::Create(dstAddr + Memory::ClampValidSizeAt(dstAddr, dstSize)); DEBUG_LOG(Log::sceMisc, "sceCccSJIStoUTF8(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); ShiftJIS sjis(src); @@ -278,7 +293,7 @@ static int sceCccSJIStoUTF16(u32 dstAddr, u32 dstSize, u32 srcAddr) { const auto src = PSPConstCharPointer::Create(srcAddr); auto dst = PSPWCharPointer::Create(dstAddr); - if (!dst.IsValid() || !src.IsValid()) + if (!dst.IsValid() || !src.IsValid() || !SourceTerminated(src)) { ERROR_LOG(Log::sceMisc, "sceCccSJIStoUTF16(%08x, %d, %08x): invalid pointers", dstAddr, dstSize, srcAddr); return 0; @@ -289,7 +304,7 @@ static int sceCccSJIStoUTF16(u32 dstAddr, u32 dstSize, u32 srcAddr) return 0; } - const auto dstEnd = PSPWCharPointer::Create(dstAddr + (dstSize & ~1)); + const auto dstEnd = PSPWCharPointer::Create(dstAddr + (Memory::ClampValidSizeAt(dstAddr, dstSize) & ~1)); DEBUG_LOG(Log::sceMisc, "sceCccSJIStoUTF16(%08x, %d, %08x)", dstAddr, dstSize, srcAddr); ShiftJIS sjis(src); diff --git a/Core/HLE/sceChnnlsv.cpp b/Core/HLE/sceChnnlsv.cpp index a1d2a373c6..fb04d388d0 100644 --- a/Core/HLE/sceChnnlsv.cpp +++ b/Core/HLE/sceChnnlsv.cpp @@ -223,6 +223,9 @@ static int sub_17A8(KirkState *kirk, u8* data) static int sceSdGetLastIndex(u32 addressCtx, u32 addressHash, u32 addressKey) { auto ctx = PSPPointer::Create(addressCtx); + // The hash and key are 16 bytes each. + if (!Memory::IsValidRange(addressHash, 16) || (addressKey && !Memory::IsValidRange(addressKey, 16))) + return hleLogError(Log::sceMisc, 0, "Invalid pointer"); u8 *hash = Memory::GetPointerWriteOrException(addressHash); if (!ctx.IsValid() || !hash) return hleLogError(Log::sceMisc, 0, "Invalid pointer"); @@ -354,7 +357,7 @@ int sceSdMacInit(pspChnnlsvContext1& ctx, int value) static int sceSdRemoveValue(u32 addressCtx, u32 addressData, int length) { auto ctx = PSPPointer::Create(addressCtx); - if (!ctx.IsValid() || !Memory::IsValidAddress(addressData)) + if (!ctx.IsValid() || !Memory::IsValidAddress(addressData) || length < 0 || !Memory::IsValidRange(addressData, (u32)length)) return hleLogError(Log::sceMisc, 0, "Invalid pointer"); return hleLogDebug(Log::sceMisc, sceSdMacUpdate(*ctx, Memory::GetPointerWriteOrException(addressData), length)); } @@ -405,6 +408,8 @@ int sceSdMacUpdate(pspChnnlsvContext1& ctx, const u8* data, int length) static int sceSdCreateList(u32 ctx2Addr, int mode, int unkwn, u32 dataAddr, u32 cryptkeyAddr) { auto ctx2 = PSPPointer::Create(ctx2Addr); + if (!Memory::IsValidRange(dataAddr, 16) || (cryptkeyAddr && !Memory::IsValidRange(cryptkeyAddr, 16))) + return hleLogError(Log::sceMisc, 0, "Invalid pointer"); u8* data = Memory::GetPointerWriteOrException(dataAddr); // Optional, as in sceSdGetLastIndex. u8* cryptkey = cryptkeyAddr ? Memory::GetPointerWriteOrException(cryptkeyAddr) : nullptr; @@ -471,6 +476,9 @@ int sceSdCipherInit(pspChnnlsvContext2& ctx2, int mode, int uknw, u8* data, cons static int sceSdSetMember(u32 ctxAddr, u32 dataAddr, int alignedLen) { auto ctx = PSPPointer::Create(ctxAddr); + // The data is decrypted in place. + if (alignedLen < 0 || !Memory::IsValidRange(dataAddr, (u32)alignedLen)) + return hleLogError(Log::sceMisc, 0, "Invalid pointer"); u8 *data = Memory::GetPointerWriteOrException(dataAddr); if (!ctx.IsValid() || !data) return hleLogError(Log::sceMisc, 0, "Invalid pointer"); diff --git a/Core/HLE/sceDeflt.cpp b/Core/HLE/sceDeflt.cpp index 4f1aa57769..3753137a71 100644 --- a/Core/HLE/sceDeflt.cpp +++ b/Core/HLE/sceDeflt.cpp @@ -15,6 +15,8 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include + #include "zlib.h" #include "Common/CommonTypes.h" @@ -41,7 +43,8 @@ static int CommonDecompress(int windowBits, u32 OutBuffer, int OutBufferLength, // We don't know the available length, just let it use as much as it wants. stream.avail_in = (uInt)Memory::ClampValidSizeAt(InBuffer, Memory::g_MemorySize); stream.next_out = outBufferPtr; - stream.avail_out = (uInt)OutBufferLength; + // The game's length isn't checked against memory anywhere else. + stream.avail_out = (uInt)Memory::ClampValidSizeAt(OutBuffer, (u32)std::max(OutBufferLength, 0)); int err = inflateInit2(&stream, windowBits); if (err != Z_OK) { diff --git a/Core/HLE/sceMd5.cpp b/Core/HLE/sceMd5.cpp index 8e7f0ff4f9..00a163d3b3 100644 --- a/Core/HLE/sceMd5.cpp +++ b/Core/HLE/sceMd5.cpp @@ -31,7 +31,7 @@ // Not really sure where these belong - is it worth giving them their own file? u32 sceKernelUtilsMt19937Init(u32 ctx, u32 seed) { DEBUG_LOG(Log::HLE, "sceKernelUtilsMt19937Init(%08x, %08x)", ctx, seed); - if (!Memory::IsValidAddress(ctx)) + if (!Memory::IsValidRange(ctx, sizeof(MersenneTwister))) return -1; void *ptr = Memory::GetPointerWriteOrException(ctx); // This is made to match the memory layout of a PSP MT structure exactly. @@ -42,7 +42,7 @@ u32 sceKernelUtilsMt19937Init(u32 ctx, u32 seed) { u32 sceKernelUtilsMt19937UInt(u32 ctx) { VERBOSE_LOG(Log::HLE, "sceKernelUtilsMt19937UInt(%08x)", ctx); - if (!Memory::IsValidAddress(ctx)) + if (!Memory::IsValidRange(ctx, sizeof(MersenneTwister))) return -1; MersenneTwister *mt = (MersenneTwister *)Memory::GetPointerUnchecked(ctx); return mt->R32(); @@ -161,7 +161,7 @@ static int Md5BlockResult(u32 ctxAddr, u32 digestAddr) { static int sceMd5Digest(u32 dataAddr, u32 len, u32 digestAddr) { DEBUG_LOG(Log::HLE, "sceMd5Digest(%08x, %d, %08x)", dataAddr, len, digestAddr); - if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidAddress(digestAddr)) + if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidRange(dataAddr, (u32)len) || !Memory::IsValidRange(digestAddr, 16)) return -1; ppsspp_md5(Memory::GetPointerWriteUnchecked(dataAddr), (int)len, Memory::GetPointerWriteUnchecked(digestAddr)); @@ -183,7 +183,7 @@ static int sceMd5BlockResult(u32 ctxAddr, u32 digestAddr) { int sceKernelUtilsMd5Digest(u32 dataAddr, int len, u32 digestAddr) { DEBUG_LOG(Log::HLE, "sceKernelUtilsMd5Digest(%08x, %d, %08x)", dataAddr, len, digestAddr); - if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidAddress(digestAddr)) + if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidRange(dataAddr, (u32)len) || !Memory::IsValidRange(digestAddr, 16)) return -1; ppsspp_md5(Memory::GetPointerWriteUnchecked(dataAddr), (int)len, Memory::GetPointerWriteUnchecked(digestAddr)); @@ -262,7 +262,7 @@ static int Sha1BlockInit(u32 ctxAddr) { int sceKernelUtilsSha1Digest(u32 dataAddr, int len, u32 digestAddr) { DEBUG_LOG(Log::HLE, "sceKernelUtilsSha1Digest(%08x, %d, %08x)", dataAddr, len, digestAddr); - if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidAddress(digestAddr)) + if (!Memory::IsValidAddress(dataAddr) || !Memory::IsValidRange(dataAddr, (u32)len) || !Memory::IsValidRange(digestAddr, 20)) return -1; sha1(Memory::GetPointerWriteUnchecked(dataAddr), (int)len, Memory::GetPointerWriteUnchecked(digestAddr)); diff --git a/Core/HLE/sceMt19937.cpp b/Core/HLE/sceMt19937.cpp index 70a50f1b5a..1f1822905c 100644 --- a/Core/HLE/sceMt19937.cpp +++ b/Core/HLE/sceMt19937.cpp @@ -31,7 +31,7 @@ static u32 sceMt19937Init(u32 mt19937Addr, u32 seed) { - if (!Memory::IsValidAddress(mt19937Addr)) + if (!Memory::IsValidRange(mt19937Addr, sizeof(MersenneTwister))) return hleLogError(Log::HLE, -1); void *ptr = Memory::GetPointerWriteUnchecked(mt19937Addr); // This is made to match the memory layout of a PSP MT structure exactly. @@ -42,7 +42,7 @@ static u32 sceMt19937Init(u32 mt19937Addr, u32 seed) static u32 sceMt19937UInt(u32 mt19937Addr) { - if (!Memory::IsValidAddress(mt19937Addr)) + if (!Memory::IsValidRange(mt19937Addr, sizeof(MersenneTwister))) return hleLogError(Log::HLE, -1); MersenneTwister *mt = (MersenneTwister *)Memory::GetPointerOrException(mt19937Addr); return hleLogVerbose(Log::HLE, mt->R32()); diff --git a/Core/HLE/sceRtc.cpp b/Core/HLE/sceRtc.cpp index 420cb863ca..76c4855213 100644 --- a/Core/HLE/sceRtc.cpp +++ b/Core/HLE/sceRtc.cpp @@ -451,7 +451,7 @@ static u32 sceRtcIsLeapYear(u32 year) static int sceRtcConvertLocalTimeToUTC(u32 tickLocalPtr,u32 tickUTCPtr) { DEBUG_LOG(Log::sceRtc, "sceRtcConvertLocalTimeToUTC(%d, %d)", tickLocalPtr, tickUTCPtr); - if (Memory::IsValidRange(tickLocalPtr, 8) && Memory::IsValidAddress(tickUTCPtr)) + if (Memory::IsValidRange(tickLocalPtr, 8) && Memory::IsValidRange(tickUTCPtr, 8)) { u64 srcTick = Memory::ReadUnchecked_U64(tickLocalPtr); // TODO : Let the user select his timezone / daylight saving instead of taking system param ? @@ -606,7 +606,7 @@ static int sceRtcGetDosTime(u32 datePtr, u32 dosTime) { } static int sceRtcSetWin32FileTime(u32 datePtr, u64 win32Time) { - if (!Memory::IsValidAddress(datePtr)) + if (!Memory::IsValidRange(datePtr, sizeof(ScePspDateTime))) { ERROR_LOG_REPORT(Log::sceRtc, "sceRtcSetWin32FileTime(%08x, %lld): invalid address", datePtr, win32Time); return -1; @@ -621,7 +621,7 @@ static int sceRtcSetWin32FileTime(u32 datePtr, u64 win32Time) { } static int sceRtcGetWin32FileTime(u32 datePtr, u32 win32TimePtr) { - if (!Memory::IsValidAddress(datePtr)) { + if (!Memory::IsValidRange(datePtr, sizeof(ScePspDateTime))) { ERROR_LOG_REPORT(Log::sceRtc, "sceRtcGetWin32FileTime(%08x, %08x): invalid address", datePtr, win32TimePtr); return -1; } @@ -917,7 +917,7 @@ static bool rtcParseRFC2822(const char *s, RtcParseResult &r) { static int sceRtcParseDateTime(u32 destTickPtr, u32 dateStringPtr) { - if (!Memory::IsValid4AlignedRange(destTickPtr, 8) || !Memory::IsValidAddress(dateStringPtr)) + if (!Memory::IsValid4AlignedRange(destTickPtr, 8) || !Memory::IsValidNullTerminatedString(dateStringPtr)) return hleLogError(Log::sceRtc, -1, "bad address"); const char *s = Memory::GetCharPointer(dateStringPtr); @@ -1052,7 +1052,7 @@ static int __RtcFormatRFC3339(u32 outPtr, u32 srcTickPtr, int tz) { static int sceRtcFormatRFC2822(u32 outPtr, u32 srcTickPtr, int tz) { - if (!Memory::IsValidAddress(outPtr) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) + if (!Memory::IsValidRange(outPtr, 32) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) { // TODO: Not well tested. ERROR_LOG(Log::sceRtc, "sceRtcFormatRFC2822(%08x, %08x, %d): invalid address", outPtr, srcTickPtr, tz); @@ -1065,7 +1065,7 @@ static int sceRtcFormatRFC2822(u32 outPtr, u32 srcTickPtr, int tz) static int sceRtcFormatRFC2822LocalTime(u32 outPtr, u32 srcTickPtr) { - if (!Memory::IsValidAddress(outPtr) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) + if (!Memory::IsValidRange(outPtr, 32) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) { // TODO: Not well tested. ERROR_LOG(Log::sceRtc, "sceRtcFormatRFC2822LocalTime(%08x, %08x): invalid address", outPtr, srcTickPtr); @@ -1089,7 +1089,7 @@ static int sceRtcFormatRFC2822LocalTime(u32 outPtr, u32 srcTickPtr) static int sceRtcFormatRFC3339(u32 outPtr, u32 srcTickPtr, int tz) { - if (!Memory::IsValidAddress(outPtr) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) + if (!Memory::IsValidRange(outPtr, 32) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) { // TODO: Not well tested. ERROR_LOG(Log::sceRtc, "sceRtcFormatRFC3339(%08x, %08x, %d): invalid address", outPtr, srcTickPtr, tz); @@ -1102,7 +1102,7 @@ static int sceRtcFormatRFC3339(u32 outPtr, u32 srcTickPtr, int tz) static int sceRtcFormatRFC3339LocalTime(u32 outPtr, u32 srcTickPtr) { - if (!Memory::IsValidAddress(outPtr) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) + if (!Memory::IsValidRange(outPtr, 32) || !Memory::IsValid4AlignedRange(srcTickPtr, 8)) { // TODO: Not well tested. ERROR_LOG(Log::sceRtc, "sceRtcFormatRFC3339LocalTime(%08x, %08x): invalid address", outPtr, srcTickPtr); diff --git a/Core/HLE/sceSfmt19937.cpp b/Core/HLE/sceSfmt19937.cpp index ff1414a923..bd0170d406 100644 --- a/Core/HLE/sceSfmt19937.cpp +++ b/Core/HLE/sceSfmt19937.cpp @@ -26,6 +26,16 @@ #include "Core/HLE/sceSfmt19937.h" +// The whole state struct is in game memory. +static bool ValidSfmt(u32 sfmt) { + return Memory::IsValidRange(sfmt, sizeof(sfmt_t)); +} + +// In 64 bits, since count * size wraps (and the end can straddle a gap between regions). +static bool ValidArray(u32 addr, int count, int elemSize) { + return count > 0 && (u64)count * elemSize <= 0xFFFFFFFF && Memory::IsValidRange(addr, (u32)((u64)count * elemSize)); +} + // The library indexes its state with idx, which lives in game memory. static bool ValidSfmtIndex(u32 sfmt) { const sfmt_t *psfmt = (const sfmt_t *)Memory::GetPointerUnchecked(sfmt); @@ -33,7 +43,7 @@ static bool ValidSfmtIndex(u32 sfmt) { } static int sceSfmt19937InitGenRand(u32 sfmt, u32 seed) { - if (!Memory::IsValidAddress(sfmt)) { + if (!ValidSfmt(sfmt)) { ERROR_LOG(Log::HLE, "sceSfmt19937InitGenRand(sfmt=%08x, seed=%08x) - bad address(es)", sfmt, seed); return -1; } @@ -46,7 +56,7 @@ static int sceSfmt19937InitGenRand(u32 sfmt, u32 seed) { } static int sceSfmt19937InitByArray(u32 sfmt, u32 seeds, int seedslen) { - if (!Memory::IsValidAddress(sfmt) || !Memory::IsValidAddress(seeds) || !Memory::IsValidAddress(seeds + 4 * (seedslen - 1))) { + if (!ValidSfmt(sfmt) || !ValidArray(seeds, seedslen, 4)) { ERROR_LOG(Log::HLE, "sceSfmt19937InitByArray(sfmt=%08x, seeds=%08x, seedslen=%08x) - bad address(es)", sfmt, seeds, seedslen); return -1; } @@ -60,7 +70,7 @@ static int sceSfmt19937InitByArray(u32 sfmt, u32 seeds, int seedslen) { } static u32 sceSfmt19937GenRand32(u32 sfmt) { - if (!Memory::IsValidAddress(sfmt) || !ValidSfmtIndex(sfmt)) { + if (!ValidSfmt(sfmt) || !ValidSfmtIndex(sfmt)) { ERROR_LOG(Log::HLE, "sceSfmt19937GenRand32(sfmt=%08x) - bad address(es)", sfmt); return -1; } @@ -73,7 +83,7 @@ static u32 sceSfmt19937GenRand32(u32 sfmt) { } static u64 sceSfmt19937GenRand64(u32 sfmt) { - if (!Memory::IsValidAddress(sfmt) || !ValidSfmtIndex(sfmt)) { + if (!ValidSfmt(sfmt) || !ValidSfmtIndex(sfmt)) { ERROR_LOG(Log::HLE, "sceSfmt19937GenRand64(sfmt=%08x) - bad address(es)", sfmt); return -1; } @@ -87,7 +97,7 @@ static u64 sceSfmt19937GenRand64(u32 sfmt) { static int sceSfmt19937FillArray32(u32 sfmt, u32 array, int arraylen) { // The library fills whole blocks, and needs at least a state's worth of them. - if (arraylen < SFMT_N32 || (arraylen % 4) != 0 || !Memory::IsValidAddress(sfmt) || !Memory::IsValidAddress(array) || !Memory::IsValidAddress(array + 4 * (arraylen - 1))) { + if (arraylen < SFMT_N32 || (arraylen % 4) != 0 || !ValidSfmt(sfmt) || !ValidArray(array, arraylen, 4)) { ERROR_LOG(Log::HLE, "sceSfmt19937FillArray32(sfmt=%08x, ar=%08x, arlen=%08x) - bad address(es)", sfmt, array, arraylen); return -1; } @@ -102,7 +112,7 @@ static int sceSfmt19937FillArray32(u32 sfmt, u32 array, int arraylen) { static int sceSfmt19937FillArray64(u32 sfmt, u32 array, int arraylen) { // The library fills whole blocks, and needs at least a state's worth of them. - if (arraylen < SFMT_N64 || (arraylen % 2) != 0 || !Memory::IsValidAddress(sfmt) || !Memory::IsValidAddress(array) || !Memory::IsValidAddress(array + 8 * (arraylen - 1))) { + if (arraylen < SFMT_N64 || (arraylen % 2) != 0 || !ValidSfmt(sfmt) || !ValidArray(array, arraylen, 8)) { ERROR_LOG(Log::HLE, "sceSfmt19937FillArray64(sfmt=%08x, ar=%08x, arlen=%08x) - bad address(es)", sfmt, array, arraylen); return -1; } diff --git a/Core/HLE/sceSha256.cpp b/Core/HLE/sceSha256.cpp index a24f4afc0a..d28dc08192 100644 --- a/Core/HLE/sceSha256.cpp +++ b/Core/HLE/sceSha256.cpp @@ -22,7 +22,7 @@ #include "Core/HLE/sceSha256.h" static int sceSha256Digest(u32 data, int dataLen, u32 digestPtr) { - if (!Memory::IsValidAddress(data) || !Memory::IsValidAddress(digestPtr) || !Memory::IsValidAddress(data + dataLen)) { + if (dataLen < 0 || !Memory::IsValidAddress(data) || !Memory::IsValidRange(data, (u32)dataLen) || !Memory::IsValidRange(digestPtr, 32)) { ERROR_LOG(Log::HLE, "sceSha256Digest(data=%08x, len=%d, digest=%08x) - bad address(es)", data, dataLen, digestPtr); return -1; } From 94469b7592b74bd8194779c5d401101af8afd015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 12:38:42 +0100 Subject: [PATCH 3/6] GE, display: Range-check contexts and display strides - sceGeSaveContext/RestoreContext and list contexts from sceGeListEnQueue save and restore 512 words, but only their first bytes were checked. - The display range check computed (271 * stride + 480) * bpp in 32 bits. sceDisplaySetFramebuf allows any multiple of 64, so a huge stride wrapped the size to a few bytes, passed, and the display was then read far past memory. Compute it in 64 bits, here and in the software renderer, which also checks its debug framebuffer copy (used for screenshots) and doesn't create textures for strides over 4096. - Frame dump replays check a DISPLAY command like sceDisplaySetFramebuf, and bound the drawn-area tracking a crafted dump could make quadratic. Co-Authored-By: Claude Opus 5.5 --- Core/HLE/sceGe.cpp | 5 +++-- GPU/Common/FramebufferManagerCommon.cpp | 6 ++++-- GPU/Debugger/Playback.cpp | 22 ++++++++++++++++++++++ GPU/GPUCommon.cpp | 3 ++- GPU/Software/SoftGpu.cpp | 7 ++++++- 5 files changed, 37 insertions(+), 6 deletions(-) diff --git a/Core/HLE/sceGe.cpp b/Core/HLE/sceGe.cpp index f9abd35ed3..511aff44d4 100644 --- a/Core/HLE/sceGe.cpp +++ b/Core/HLE/sceGe.cpp @@ -605,7 +605,8 @@ u32 sceGeSaveContext(u32 ctxAddr) { } // Let's just dump gstate. - if (Memory::IsValidAddress(ctxAddr)) { + // A context (SceGeContext) is 512 words. + if (Memory::IsValidRange(ctxAddr, 512 * 4)) { gstate.Save((u32_le *)Memory::GetPointerOrException(ctxAddr)); } @@ -619,7 +620,7 @@ u32 sceGeRestoreContext(u32 ctxAddr) { return hleLogWarning(Log::sceGe, SCE_KERNEL_ERROR_BUSY, "lists in process, aborting"); } - if (Memory::IsValidAddress(ctxAddr)) { + if (Memory::IsValidRange(ctxAddr, 512 * 4)) { gstate.Restore((u32_le *)Memory::GetPointerOrException(ctxAddr)); } diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 0a909fa5b3..8c2f380281 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -1517,8 +1517,10 @@ static bool IsValidDisplayRange(u32 addr, int stride, GEBufferFormat format) { if (stride <= 0) { return false; } - const u32 bpp = BufferFormatBytesPerPixel(format); - return Memory::IsValidRange(addr, ((272 - 1) * (u32)stride + 480) * bpp); + // In 64 bits: a huge stride (a multiple of 64 is all sceDisplaySetFramebuf asks for) wrapped + // this to a small size that passed. + const u64 bytes = ((272 - 1) * (u64)stride + 480) * BufferFormatBytesPerPixel(format); + return bytes <= 0xFFFFFFFF && Memory::IsValidRange(addr, (u32)bytes); } Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, GEBufferFormat srcPixelFormat, int srcStride, int width, int height) { diff --git a/GPU/Debugger/Playback.cpp b/GPU/Debugger/Playback.cpp index 25f59d6c49..70444ef323 100644 --- a/GPU/Debugger/Playback.cpp +++ b/GPU/Debugger/Playback.cpp @@ -778,7 +778,13 @@ void DumpExecute::MarkDrawn(u32 prim) { return; } + // Real frames draw to a few targets with a few distinct areas, but a crafted dump can make every + // prim new, and each one is checked against all the others. Past these, coarsen instead. + const size_t MAX_TARGETS = 1024; + const size_t MAX_RECTS = 64; auto add = [&](u32 base, u32 strideBytes, u32 bytesPerPixel, const DrawnRect &rect) { + if (drawnTargets_.size() >= MAX_TARGETS && drawnTargets_.find(base) == drawnTargets_.end()) + return; DrawnTarget &t = drawnTargets_[base]; t.strideBytes = strideBytes; t.bpp = bytesPerPixel; @@ -786,6 +792,16 @@ void DumpExecute::MarkDrawn(u32 prim) { if (r.Contains(rect.x1, rect.y1) && r.Contains(rect.x2, rect.y2)) return; } + if (t.rects.size() >= MAX_RECTS) { + // Merge everything into one bounding box. That can only skip more in CopyAroundDrawn. + DrawnRect u = rect; + for (const DrawnRect &r : t.rects) { + u = DrawnRect{ std::min(u.x1, r.x1), std::min(u.y1, r.y1), std::max(u.x2, r.x2), std::max(u.y2, r.y2) }; + } + t.rects.clear(); + t.rects.push_back(u); + return; + } t.rects.push_back(rect); }; add(start, (fbWidth_ & 0x07FC) * bpp, bpp, DrawnRect{ x1, y1, x2, y2 }); @@ -1116,6 +1132,12 @@ void DumpExecute::Display(u32 ptr, u32 sz, bool allowFlip) { haveDisplay_ = disp->topaddr.ptr != 0; if (!haveDisplay_) return; + // The dump is a file, so check what sceDisplaySetFramebuf would have. + if ((disp->topaddr.ptr & 0xF) != 0 || disp->linesize <= 0 || (disp->linesize & 0x3F) != 0 || disp->linesize > 4096 || disp->pixelFormat < 0 || disp->pixelFormat > GE_FORMAT_8888) { + ERROR_LOG(Log::G3D, "Bad display in dump: %08x, stride %d, format %d", disp->topaddr.ptr, disp->linesize, disp->pixelFormat); + haveDisplay_ = false; + return; + } __DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 1); if (allowFlip) { __DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 0); diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index 428ffc4c11..b3cc32b8bf 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -624,7 +624,8 @@ u32 GPUCommon::EnqueueList(u32 listpc, u32 stall, int subIntrBase, PSPPointercontext.IsValid()) + // The whole 512-word context is saved and restored there, not just what PSPPointer checks. + if (args.IsValid() && args->context.IsValid() && Memory::IsValidRange(args->context.ptr, 512 * 4)) dl.context = args->context; else dl.context = 0; diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp index 0ed0ec1fe3..64e28389c7 100644 --- a/GPU/Software/SoftGpu.cpp +++ b/GPU/Software/SoftGpu.cpp @@ -647,7 +647,8 @@ void SoftGPU::CopyToCurrentFboFromDisplayRam(const DisplayLayoutConfig &config, outputFlags |= OutputFlags::PILLARBOX; } } - } else if (srcwidth == 0 || srcheight == 0 || displayStride_ < 0 || !Memory::IsValidRange(displayFramebuf_, (displayStride_ == 0 ? srcwidth : displayStride_) * srcheight * (displayFormat_ == GE_FORMAT_8888 ? 4 : 2))) { + } else if (srcwidth == 0 || srcheight == 0 || displayStride_ > 4096 || (u64)(displayStride_ == 0 ? srcwidth : displayStride_) * srcheight * (displayFormat_ == GE_FORMAT_8888 ? 4 : 2) > 0xFFFFFFFF || + !Memory::IsValidRange(displayFramebuf_, (displayStride_ == 0 ? srcwidth : displayStride_) * srcheight * (displayFormat_ == GE_FORMAT_8888 ? 4 : 2))) { // Nothing, or not all of it in memory: the PSP takes strides that run past the end of VRAM, or // negative ones (rows going up.) hasImage = false; @@ -1604,6 +1605,10 @@ bool SoftGPU::GetCurrentFramebuffer(GPUDebugBuffer &buffer, GPUDebugFramebufferT size.y = 272; stride = displayStride_; fmt = displayFormat_; + // All 272 rows are copied, at the game's stride. + const u64 bytes = ((u64)(size.y - 1) * stride + size.x) * (fmt == GE_FORMAT_8888 ? 4 : 2); + if (bytes > 0xFFFFFFFF || !Memory::IsValidRange(displayFramebuf_, (u32)bytes)) + return false; src = Memory::GetPointerOrException(displayFramebuf_); } From 917607d8faa740c273ea9c12386d042cdf6b82ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 12:43:20 +0100 Subject: [PATCH 4/6] sceIo, savedata, fonts: Check whole structs and arrays, not first bytes - Block-mode sceIoRead counts its size in sectors, but was clamped to memory in bytes, so a large count read size * 2048 bytes past the buffer. - sceIoDread wrote the short and long names to d_private after checking one byte; the PGD key and sector seek ioctls read 16 bytes after one. - Savedata: GetSize walked the game's entry arrays by its counts (unchecked), GetList and GetFilesList wrote entries past the first unchecked, and Save read the data and icon/pic/sound buffers by their sizes without checking them. Also keep the encrypted size from going negative. - sceFont: font style, font info and char info structs, and the font list, were only checked at their first byte. Style names in logs needn't be terminated. - PGF glyph rendering wrote 3-4 bytes per pixel after checking one. Co-Authored-By: Claude Opus 5.5 --- Core/Dialog/SavedataParam.cpp | 39 ++++++++++++++++++++++++++++++----- Core/Font/PGF.cpp | 2 +- Core/HLE/sceFont.cpp | 16 +++++++------- Core/HLE/sceIo.cpp | 22 +++++++++++++------- 4 files changed, 59 insertions(+), 20 deletions(-) diff --git a/Core/Dialog/SavedataParam.cpp b/Core/Dialog/SavedataParam.cpp index 299ec4d351..39d31acabb 100644 --- a/Core/Dialog/SavedataParam.cpp +++ b/Core/Dialog/SavedataParam.cpp @@ -422,11 +422,21 @@ int SavedataParam::Save(SceUtilitySavedataParam* param, const std::string &saveD ERROR_LOG_REPORT(Log::sceUtility, "Savedata buffer overflow: %d / %d", param->dataSize, param->dataBufSize); return SCE_UTILITY_SAVEDATA_ERROR_RW_BAD_PARAMS; } + // Everything below reads these buffers by their sizes, so they have to be in memory. A size of + // 2^31 or more would also go negative as the int the encryption uses. + if (param->dataBuf.IsValid() && (param->dataSize > 0x7FFFFFF0 || !Memory::IsValidRange(param->dataBuf.ptr, param->dataSize))) { + ERROR_LOG_REPORT(Log::sceUtility, "Savedata buffer %08x (%d bytes) isn't all in memory", param->dataBuf.ptr, param->dataSize); + return SCE_UTILITY_SAVEDATA_ERROR_RW_BAD_PARAMS; + } auto validateSize = [](const PspUtilitySavedataFileData &data) { if (data.buf.IsValid() && data.bufSize < data.size) { ERROR_LOG_REPORT(Log::sceUtility, "Savedata subdata buffer overflow: %d / %d", data.size, data.bufSize); return false; } + if (data.buf.IsValid() && !Memory::IsValidRange(data.buf.ptr, data.size)) { + ERROR_LOG_REPORT(Log::sceUtility, "Savedata subdata buffer %08x (%d bytes) isn't all in memory", data.buf.ptr, data.size); + return false; + } return true; }; if (!validateSize(param->icon0FileData) || !validateSize(param->icon1FileData) || !validateSize(param->pic1FileData) || !validateSize(param->snd0FileData)) { @@ -462,7 +472,7 @@ int SavedataParam::Save(SceUtilitySavedataParam* param, const std::string &saveD cryptedSize = param->dataSize; if (cryptedSize == 0 || (SceSize)cryptedSize > param->dataBufSize) { ERROR_LOG(Log::sceUtility, "Bad cryptedSize %d", cryptedSize); - cryptedSize = param->dataBufSize; // fallback, should never use this + cryptedSize = (int)std::min(Memory::ClampValidSizeAt(param->dataBuf.ptr, param->dataBufSize), (u32)0x7FFFFFF0); // fallback, should never use this } u8 *data_ = param->dataBuf; @@ -568,7 +578,7 @@ int SavedataParam::Save(SceUtilitySavedataParam* param, const std::string &saveD { saveSize = param->dataSize; if(saveSize == 0 || saveSize > param->dataBufSize) - saveSize = param->dataBufSize; // fallback, should never use this + saveSize = Memory::ClampValidSizeAt(param->dataBuf.ptr, param->dataBufSize); // fallback, should never use this data_ = param->dataBuf; } @@ -1117,6 +1127,13 @@ inline std::string FmtPspTime(const ScePspDateTime &dt) { return StringFromFormat("%04d-%02d-%02d %02d:%02d:%02d.%06d", dt.year, dt.month, dt.day, dt.hour, dt.minute, dt.second, dt.microsecond); } +// Whether entry `index` of a guest array is all in memory (PSPPointer only checks the first). +template +static bool EntryInRange(const PSPPointer &entries, u32 index) { + const u64 end = (u64)entries.ptr + ((u64)index + 1) * sizeof(T); + return end <= 0x100000000ULL && Memory::IsValidRange(entries.ptr, (u32)(end - entries.ptr)); +} + int SavedataParam::GetSizes(SceUtilitySavedataParam *param) { if (!param) { return SCE_UTILITY_SAVEDATA_ERROR_SIZES_NO_DATA; @@ -1246,6 +1263,10 @@ bool SavedataParam::GetList(SceUtilitySavedataParam *param) sfoFiles.push_back(sfoFile); } + if (!validDir.empty() && !EntryInRange(param->idList->entries, (u32)validDir.size() - 1)) { + ERROR_LOG(Log::sceUtility, "SavedataParam::GetList(): entries %08x can't hold %d", param->idList->entries.ptr, (int)validDir.size()); + return false; + } SceUtilitySavedataIdListEntry *entries = param->idList->entries; for (u32 i = 0; i < (u32)validDir.size(); i++) { @@ -1361,11 +1382,11 @@ int SavedataParam::GetFilesList(SceUtilitySavedataParam *param, u32 requestAddr, SceUtilitySavedataFileListEntry *entry = NULL; int sizeOffset = 0; if (isSystemFile) { - if (fileList->systemEntries.IsValid() && fileList->resultNumSystemEntries < fileList->maxSystemEntries) { + if (fileList->systemEntries.IsValid() && fileList->resultNumSystemEntries < fileList->maxSystemEntries && EntryInRange(fileList->systemEntries, fileList->resultNumSystemEntries)) { entry = &fileList->systemEntries[fileList->resultNumSystemEntries++]; } } else if (secureFilenames.find(file.name) != secureFilenames.end()) { - if (fileList->secureEntries.IsValid() && fileList->resultNumSecureEntries < fileList->maxSecureEntries) { + if (fileList->secureEntries.IsValid() && fileList->resultNumSecureEntries < fileList->maxSecureEntries && EntryInRange(fileList->secureEntries, fileList->resultNumSecureEntries)) { entry = &fileList->secureEntries[fileList->resultNumSecureEntries++]; } // Secure files are slightly bigger. @@ -1373,7 +1394,7 @@ int SavedataParam::GetFilesList(SceUtilitySavedataParam *param, u32 requestAddr, sizeOffset = -0x10; } } else { - if (fileList->normalEntries.IsValid() && fileList->resultNumNormalEntries < fileList->maxNormalEntries) { + if (fileList->normalEntries.IsValid() && fileList->resultNumNormalEntries < fileList->maxNormalEntries && EntryInRange(fileList->normalEntries, fileList->resultNumNormalEntries)) { entry = &fileList->normalEntries[fileList->resultNumNormalEntries++]; } } @@ -1437,6 +1458,14 @@ bool SavedataParam::GetSize(SceUtilitySavedataParam *param) { auto listing = pspFileSystem.GetDirListing(saveDir, &exists); const u64 freeBytes = MemoryStick_FreeSpace(GetGameName(param)); + // The counts and both arrays come from the game. + if (param->sizeInfo->numNormalEntries < 0 || param->sizeInfo->numSecureEntries < 0 || + (param->sizeInfo->numNormalEntries > 0 && !EntryInRange(param->sizeInfo->normalEntries, param->sizeInfo->numNormalEntries - 1)) || + (param->sizeInfo->numSecureEntries > 0 && !EntryInRange(param->sizeInfo->secureEntries, param->sizeInfo->numSecureEntries - 1))) { + ERROR_LOG(Log::sceUtility, "SavedataParam::GetSize(): bad entry lists"); + return false; + } + s64 overwriteBytes = 0; s64 writeBytes = 0; for (int i = 0; i < param->sizeInfo->numNormalEntries; ++i) { diff --git a/Core/Font/PGF.cpp b/Core/Font/PGF.cpp index 7f6a200470..35f2ee86c3 100644 --- a/Core/Font/PGF.cpp +++ b/Core/Font/PGF.cpp @@ -779,7 +779,7 @@ void PGF::SetFontPixel(u32 base, int bpl, int bufWidth, int bufHeight, int x, in // glyph smears across them - see the "Linesize = 1" case in pspautotests font/charglyphimage. // The bufWidth/bufHeight rectangle above and the address check below are what keep this sane. int framebufferAddr = base + (y * bpl) + (pixelBytes == 0 ? x / 2 : x * pixelBytes); - if (!Memory::IsValidAddress(framebufferAddr)) { + if (!Memory::IsValidRange(framebufferAddr, pixelBytes == 0 ? 1 : pixelBytes)) { return; } diff --git a/Core/HLE/sceFont.cpp b/Core/HLE/sceFont.cpp index 0c8d225276..03a328b783 100644 --- a/Core/HLE/sceFont.cpp +++ b/Core/HLE/sceFont.cpp @@ -1316,14 +1316,14 @@ static int sceFontFindOptimumFont(u32 libHandle, u32 fontStylePtr, u32 errorCode return hleReportError(Log::sceFont, 0, "invalid font lib"); } - if (!Memory::IsValidAddress(fontStylePtr)) { + if (!Memory::IsValidRange(fontStylePtr, sizeof(PGFFontStyle))) { // Yes, actually. Must've been a typo in the library. *errorCode = SCE_FONT_ERROR_INVALID_LIBID; return hleReportError(Log::sceFont, 0, "invalid style address"); } auto requestedStyle = PSPPointer::Create(fontStylePtr); - DEBUG_LOG(Log::sceFont, "requestedStyle fontAttributes %i,fontCountry %i,fontExpire %i,fontFamily %i,fontFileName %s,fontH %f,fontHRes %f,fontLanguage %i,fontName %s,fontRegion %i,fontStyle %i,fontStyleSub %i,fontV %f,fontVRes %f,fontWeight %f", requestedStyle->fontAttributes, requestedStyle->fontCountry, requestedStyle->fontExpire, requestedStyle->fontFamily, requestedStyle->fontFileName, requestedStyle->fontH, requestedStyle->fontHRes, requestedStyle->fontLanguage, requestedStyle->fontName, requestedStyle->fontRegion, requestedStyle->fontStyle, requestedStyle->fontStyleSub, requestedStyle->fontV, requestedStyle->fontVRes, requestedStyle->fontWeight); + DEBUG_LOG(Log::sceFont, "requestedStyle fontAttributes %i,fontCountry %i,fontExpire %i,fontFamily %i,fontFileName %.64s,fontH %f,fontHRes %f,fontLanguage %i,fontName %.64s,fontRegion %i,fontStyle %i,fontStyleSub %i,fontV %f,fontVRes %f,fontWeight %f", requestedStyle->fontAttributes, requestedStyle->fontCountry, requestedStyle->fontExpire, requestedStyle->fontFamily, requestedStyle->fontFileName, requestedStyle->fontH, requestedStyle->fontHRes, requestedStyle->fontLanguage, requestedStyle->fontName, requestedStyle->fontRegion, requestedStyle->fontStyle, requestedStyle->fontStyleSub, requestedStyle->fontV, requestedStyle->fontVRes, requestedStyle->fontWeight); // Find the first nearest match for H/V, OR the last exact match for others. float hRes = requestedStyle->fontHRes > 0.0f ? (float)requestedStyle->fontHRes : fontLib->FontHRes(); @@ -1383,7 +1383,7 @@ static int sceFontFindFont(u32 libHandle, u32 fontStylePtr, u32 errorCodePtr) { return hleNoLog(0); } - if (!Memory::IsValidAddress(fontStylePtr)) { + if (!Memory::IsValidRange(fontStylePtr, sizeof(PGFFontStyle))) { ERROR_LOG(Log::sceFont, "sceFontFindFont(%08x, %08x, %08x): invalid style address", libHandle, fontStylePtr, errorCodePtr); *errorCode = SCE_FONT_ERROR_INVALID_PARAMETER; return hleNoLog(0); @@ -1392,7 +1392,7 @@ static int sceFontFindFont(u32 libHandle, u32 fontStylePtr, u32 errorCodePtr) { DEBUG_LOG(Log::sceFont, "sceFontFindFont(%x, %x, %x)", libHandle, fontStylePtr, errorCodePtr); auto requestedStyle = PSPPointer::Create(fontStylePtr); - DEBUG_LOG(Log::sceFont, "requestedStyle fontAttributes %i,fontCountry %i,fontExpire %i,fontFamily %i,fontFileName %s,fontH %f,fontHRes %f,fontLanguage %i,fontName %s,fontRegion %i,fontStyle %i,fontStyleSub %i,fontV %f,fontVRes %f,fontWeight %f", requestedStyle->fontAttributes, requestedStyle->fontCountry, requestedStyle->fontExpire, requestedStyle->fontFamily, requestedStyle->fontFileName, requestedStyle->fontH, requestedStyle->fontHRes, requestedStyle->fontLanguage, requestedStyle->fontName, requestedStyle->fontRegion, requestedStyle->fontStyle, requestedStyle->fontStyleSub, requestedStyle->fontV, requestedStyle->fontVRes, requestedStyle->fontWeight); + DEBUG_LOG(Log::sceFont, "requestedStyle fontAttributes %i,fontCountry %i,fontExpire %i,fontFamily %i,fontFileName %.64s,fontH %f,fontHRes %f,fontLanguage %i,fontName %.64s,fontRegion %i,fontStyle %i,fontStyleSub %i,fontV %f,fontVRes %f,fontWeight %f", requestedStyle->fontAttributes, requestedStyle->fontCountry, requestedStyle->fontExpire, requestedStyle->fontFamily, requestedStyle->fontFileName, requestedStyle->fontH, requestedStyle->fontHRes, requestedStyle->fontLanguage, requestedStyle->fontName, requestedStyle->fontRegion, requestedStyle->fontStyle, requestedStyle->fontStyleSub, requestedStyle->fontV, requestedStyle->fontVRes, requestedStyle->fontWeight); // Find the closest exact match for the fields specified. float hRes = requestedStyle->fontHRes > 0.0f ? (float)requestedStyle->fontHRes : fontLib->FontHRes(); @@ -1418,7 +1418,7 @@ static int sceFontFindFont(u32 libHandle, u32 fontStylePtr, u32 errorCodePtr) { } static int sceFontGetFontInfo(u32 fontHandle, u32 fontInfoPtr) { - if (!Memory::IsValidAddress(fontInfoPtr)) { + if (!Memory::IsValidRange(fontInfoPtr, sizeof(PGFFontInfo))) { return hleLogError(Log::sceFont, SCE_FONT_ERROR_INVALID_PARAMETER, "bad fontInfo pointer"); } LoadedFont *font = GetLoadedFont(fontHandle, true); @@ -1455,7 +1455,7 @@ static int sceFontGetFontInfoByIndexNumber(u32 libHandle, u32 fontInfoPtr, u32 i static int sceFontGetCharInfo(u32 fontHandle, u32 charCode, u32 charInfoPtr) { charCode &= 0xffff; - if (!Memory::IsValidAddress(charInfoPtr)) { + if (!Memory::IsValidRange(charInfoPtr, sizeof(PGFCharInfo))) { return hleLogError(Log::sceFont, SCE_FONT_ERROR_INVALID_PARAMETER, "bad charInfo pointer"); } LoadedFont *font = GetLoadedFont(fontHandle, true); @@ -1494,7 +1494,7 @@ static int sceFontGetCharInfo(u32 fontHandle, u32 charCode, u32 charInfoPtr) { static int sceFontGetShadowInfo(u32 fontHandle, u32 charCode, u32 charInfoPtr) { charCode &= 0xffff; - if (!Memory::IsValidAddress(charInfoPtr)) { + if (!Memory::IsValidRange(charInfoPtr, sizeof(PGFCharInfo))) { return hleLogError(Log::sceFont, SCE_FONT_ERROR_INVALID_PARAMETER, "bad charInfo pointer"); } LoadedFont *font = GetLoadedFont(fontHandle, true); @@ -1618,6 +1618,8 @@ static int sceFontGetFontList(u32 fontLibHandle, u32 fontStylePtr, int numFonts) DEBUG_LOG(Log::sceFont, "sceFontGetFontList(%08x, %08x, %i)", fontLibHandle, fontStylePtr, numFonts); if (fl->handle() != 0) { numFonts = std::min(numFonts, (int)internalFonts.size()); + // Only the first entry was checked above. + numFonts = std::min(numFonts, (int)(Memory::ClampValidSizeAt(fontStylePtr, (u32)std::max(numFonts, 0) * sizeof(PGFFontStyle)) / sizeof(PGFFontStyle))); for (int i = 0; i < numFonts; i++) fontStyles[i] = internalFonts[i]->GetFontStyle(); } diff --git a/Core/HLE/sceIo.cpp b/Core/HLE/sceIo.cpp index 6229e120a2..e22e35167a 100644 --- a/Core/HLE/sceIo.cpp +++ b/Core/HLE/sceIo.cpp @@ -1145,6 +1145,11 @@ static bool __IoRead(int &result, int id, u32 data_addr, int size, int &us) { NotifyMemInfo(MemBlockFlags::WRITE, data_addr, size, tag.c_str(), tag.size()); u8 *data = (u8 *)Memory::GetPointerUnchecked(data_addr); u32 validSize = Memory::ClampValidSizeAt(data_addr, size); + // Block devices count the size in sectors, so clamp what that many sectors cover. + if ((pspFileSystem.DevType(f->handle) & PSPDevType::BLOCK) != 0) { + const u32 bytes = (u32)std::min((u64)size * 2048, 0xFFFFFFFF); + validSize = Memory::ClampValidSizeAt(data_addr, bytes) / 2048; + } if (f->npdrm) { result = npdrmRead(f, data, validSize); currentMIPS->InvalidateICacheRangeDeferred(data_addr, validSize); @@ -2755,21 +2760,23 @@ static u32 sceIoDread(MIPSState *mips, int id, u32 dirent_addr) { // write d_private for supporting Custom BGM // ref JPCSP https://code.google.com/p/jpcsp/source/detail?r=3468 if (Memory::IsValidAddress(entry->d_private)){ + // What's written depends on the layout, so check each one's whole range below. if (sceKernelGetCompiledSdkVersion() <= 0x0307FFFF){ // d_private is pointing to an area of unknown size // - [0..12] "8.3" file name (null-terminated), could be empty. // - [13..???] long file name (null-terminated) - - strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private), shortName.c_str(), 13); - strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 13), (const char*)entry->d_name, ARRAY_SIZE(entry->d_name)); + if (Memory::IsValidRange(entry->d_private, 13 + (u32)ARRAY_SIZE(entry->d_name))) { + strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private), shortName.c_str(), 13); + strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 13), (const char*)entry->d_name, ARRAY_SIZE(entry->d_name)); + } } else { // d_private is pointing to an area of total size 1044 // - [0..3] size of area // - [4..19] "8.3" file name (null-terminated), could be empty. // - [20..???] long file name (null-terminated) - auto size = Memory::ReadUnchecked_U32(entry->d_private); - if (size >= 1044) { + auto size = Memory::IsValidRange(entry->d_private, 4) ? Memory::ReadUnchecked_U32(entry->d_private) : 0; + if (size >= 1044 && Memory::IsValidRange(entry->d_private, 1044)) { strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 4), shortName.c_str(), 16); strcpy_limit((char*)Memory::GetPointerUnchecked(entry->d_private + 20), (const char*)entry->d_name, ARRAY_SIZE(entry->d_name)); } @@ -3087,7 +3094,7 @@ int __IoIoctl(u32 id, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 out u8 pgd_header[0x90]{}; u8 pgd_magic[4] = {0x00, 0x50, 0x47, 0x44}; - if (Memory::IsValidAddress(indataPtr) && inlen == 16) { + if (Memory::IsValidRange(indataPtr, 16) && inlen == 16) { memcpy(keybuf, Memory::GetPointerUnchecked(indataPtr), 16); key_ptr = keybuf; }else{ @@ -3285,7 +3292,8 @@ int __IoIoctl(u32 id, u32 cmd, u32 indataPtr, u32 inlen, u32 outdataPtr, u32 out //if (GetIOTimingMethod() == IOTIMING_REALISTIC) // Need a check for io timing method? usec += 15000;// Fantasy Golf Pangya Portable(KS) needs a delay over 15000us. - if (Memory::IsValidAddress(indataPtr) && inlen >= 4) { + // The whole struct is read, whatever inlen says. + if (Memory::IsValidRange(indataPtr, 16) && inlen >= 4) { struct SeekInfo { u64_le offset; u32_le unk; From f4b99a6a99f8c2d2ff93edddb5606657df74aacb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 12:45:45 +0100 Subject: [PATCH 5/6] Ad hoc, PMP: Bound guest-sized buffers and lists - Game mode master and replica data was copied by the game's size after checking one byte, into host buffers allocated by that size (up to 2 GB). If the copy failed, the uninitialized buffer was sent to peers. - sceNetAdhocctlCreateEnterGameMode read up to 16 member MACs after checking one byte. - sceNetAdhocMatchingCreate allocated and cleared the game's receive buffer size on the host with no upper bound. Cap it at 1 MB. - PMP video walked its block descriptors with only the first one checked. Co-Authored-By: Claude Opus 5.5 --- Core/HLE/sceMpeg.cpp | 3 +++ Core/HLE/sceNetAdhoc.cpp | 8 ++++++-- Core/HLE/sceNetAdhocMatching.cpp | 5 +++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Core/HLE/sceMpeg.cpp b/Core/HLE/sceMpeg.cpp index 2784f788b9..33b82f5a9b 100644 --- a/Core/HLE/sceMpeg.cpp +++ b/Core/HLE/sceMpeg.cpp @@ -962,6 +962,9 @@ static bool decodePmpVideo(PSPPointer ringbuffer, u32 pmpctxA // joint all blocks into H264Frames for (int i = 0; i < pmp_nBlocks; i++){ auto lli = PSPPointer::Create(pmp_videoSource); + // Each block descriptor is in game memory, after the first. + if (!lli.IsValid()) + break; // add source block into pmpframes const uint8_t *ptr = Memory::GetPointerRangeOrException(lli->pSrc, lli->iSize); if (ptr) diff --git a/Core/HLE/sceNetAdhoc.cpp b/Core/HLE/sceNetAdhoc.cpp index 4ec5b5bc43..0cbcda4faa 100644 --- a/Core/HLE/sceNetAdhoc.cpp +++ b/Core/HLE/sceNetAdhoc.cpp @@ -3391,6 +3391,9 @@ int NetAdhocctl_CreateEnterGameMode(const char* group_name, int game_type, int n if (game_type < ADHOCCTL_GAMETYPE_1A || game_type > ADHOCCTL_GAMETYPE_2A || num_members < 2 || num_members > 16 || (game_type == ADHOCCTL_GAMETYPE_1A && num_members > 4)) return SCE_NET_ADHOCCTL_ERROR_INVALID_ARG; + // The whole member list is read below. + if (!Memory::IsValidRange(membersAddr, num_members * (u32)sizeof(SceNetEtherAddr))) + return SCE_NET_ADHOCCTL_ERROR_INVALID_ARG; // In a normal group, it reports ALREADY_CONNECTED through the handlers and stays in it // (pspautotests net/adhoc/gamemode). @@ -4615,7 +4618,8 @@ static int sceNetAdhocGameModeCreateMaster(MIPSState *mips, u32 dataAddr, int si if (!netAdhocGameModeEntered) return hleLogError(Log::sceNet, SCE_NET_ADHOCCTL_ERROR_NOT_ENTER_GAMEMODE, "not enter gamemode"); - if (size < 0 || !Memory::IsValidAddress(dataAddr)) + // The data is copied (and the buffers are sized) by size. + if (size < 0 || !Memory::IsValidAddress(dataAddr) || !Memory::IsValidRange(dataAddr, (u32)size)) return hleLogError(Log::sceNet, SCE_NET_ADHOCCTL_ERROR_INVALID_ARG, "invalid arg"); // pspautotests net/adhoc/pair/gamemode @@ -4672,7 +4676,7 @@ static int sceNetAdhocGameModeCreateReplica(MIPSState *mips, const char *mac, u3 if (!netAdhocGameModeEntered) return hleLogError(Log::sceNet, SCE_NET_ADHOCCTL_ERROR_NOT_ENTER_GAMEMODE, "not enter gamemode"); - if (mac == nullptr || size < 0 || !Memory::IsValidAddress(dataAddr)) + if (mac == nullptr || size < 0 || !Memory::IsValidAddress(dataAddr) || !Memory::IsValidRange(dataAddr, (u32)size)) return hleLogError(Log::sceNet, SCE_NET_ADHOCCTL_ERROR_INVALID_ARG, "invalid arg"); const SceNetEtherAddr &replicaMac = *(const SceNetEtherAddr *)mac; diff --git a/Core/HLE/sceNetAdhocMatching.cpp b/Core/HLE/sceNetAdhocMatching.cpp index fdce9166c2..687dc54dc1 100644 --- a/Core/HLE/sceNetAdhocMatching.cpp +++ b/Core/HLE/sceNetAdhocMatching.cpp @@ -1576,6 +1576,11 @@ static int sceNetAdhocMatchingCreate(MIPSState *mips, int mode, int maxnum, int // Invalid Receive Buffer Size return hleLogError(Log::sceNet, SCE_NET_ADHOC_MATCHING_ERROR_RXBUF_TOO_SHORT, "adhoc matching rxbuf too short"); } + // We allocate (and clear) this on the host. The PSP takes it from the library's small heap, and + // no packet is anywhere near this big. + if (rxbuflen > 1024 * 1024) { + return hleLogError(Log::sceNet, SCE_NET_ADHOC_MATCHING_ERROR_NO_SPACE, "adhoc matching rxbuf too large"); + } if (mode < 1 || mode > 3) { return hleLogError(Log::sceNet, SCE_NET_ADHOC_MATCHING_ERROR_INVALID_MODE, "adhoc matching invalid mode"); From 68f8c061eea6db238a6623afe4eaea865beb70e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 5 Nov 2026 14:58:46 +0100 Subject: [PATCH 6/6] docs: Note that sceNet_lib is experimental and out of scope for reviews Co-Authored-By: Claude Opus 5.5 --- docs/HLEModules.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/HLEModules.md b/docs/HLEModules.md index d4dfc4f454..18b9590044 100644 --- a/docs/HLEModules.md +++ b/docs/HLEModules.md @@ -57,3 +57,9 @@ small examples to copy from). A module is a `const HLEFunction []` table o (only `android/jni/Android.mk` and `libretro/Makefile.common` genuinely can't be build-tested here - see [Building and testing](building.md) for what verification is possible for those.) + +## Modules to leave out of reviews + +`sceNet_lib` (`Core/HLE/sceNet_lib.cpp`) is an experimental implementation of some of the firmware's +`pspnet.prx` exports, added by a contributor. Games can't call it, so leave it out of HLE reviews and +hardening passes.