From 4cd611b71fbf4e30cf6a152cbea393757d4db945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 1 Aug 2026 11:32:53 +0200 Subject: [PATCH] Fix out-of-bounds reads in ATRAC track parsing AnalyzeAtracTrack used max(fileSize, size) as the chunk-parse bound with fileSize taken from the file's RIFF header, so a crafted inflated RIFF size could push reads past the end of the buffer. Keep the real-library behavior of tolerating a too-low size, but clamp the parse bound to the actual mapped guest memory at the buffer. Also guard ParseWaveAT3's RIFF scan against a blockSize < 4 underflow that could make the offset negative and bypass the loop bounds check, and clamp readSize to the mapped region in Atrac2::SetData before parsing. --- Core/HLE/AtracCtx2.cpp | 4 +++- Core/Util/AtracTrack.cpp | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Core/HLE/AtracCtx2.cpp b/Core/HLE/AtracCtx2.cpp index 78500eff04..8a6162fd27 100644 --- a/Core/HLE/AtracCtx2.cpp +++ b/Core/HLE/AtracCtx2.cpp @@ -975,7 +975,9 @@ int Atrac2::SetData(const Track &track, u32 bufferAddr, u32 readSize, u32 buffer // Turns out that games can abuse bufferSize, so we can't verify that it's a valid length with GetPointerRange. const u8 *bufferPtr = Memory::GetPointerUnchecked(bufferAddr); if (!Memory::IsValidRange(bufferAddr, readSize)) { - WARN_LOG(Log::Atrac, "Atrac2::SetData: Bad buffer range %08x+%08x - however, proceeeding.", bufferAddr, readSize); + WARN_LOG(Log::Atrac, "Atrac2::SetData: Bad buffer range %08x+%08x - clamping to mapped size.", bufferAddr, readSize); + // Clamp so the parsers below can't read past the mapped region. + readSize = Memory::ClampValidSizeAt(bufferAddr, readSize); } if (!isAA3) { int retval = ParseWaveAT3(bufferPtr, readSize, &trackInfo); diff --git a/Core/Util/AtracTrack.cpp b/Core/Util/AtracTrack.cpp index 65044fd055..60fecd5eea 100644 --- a/Core/Util/AtracTrack.cpp +++ b/Core/Util/AtracTrack.cpp @@ -84,7 +84,14 @@ int AnalyzeAtracTrack(const u8 *buffer, u32 size, Track *track, std::string *err track->fileSize = Read32(buffer, offset - 8) + 8; // Even if the RIFF size is too low, it may simply be incorrect. This works on real firmware. + // But the reads below must stay within mapped guest memory: clamp the + // parse bound to the actual mapped region at the buffer, so a crafted, + // inflated RIFF size can't push the reads past the end of RAM. u32 maxSize = std::max(track->fileSize, size); + const u32 bufferAddr = Memory::GetAddressFromHostPointer(buffer); + if (bufferAddr != 0) { + maxSize = std::min(maxSize, Memory::MaxSizeAtAddress(bufferAddr)); + } bool bfoundData = false; u32 dataChunkSize = 0; @@ -375,6 +382,12 @@ int ParseWaveAT3(const u8 *data, u32 dataLength, TrackInfo *track) { // We found the WAVE header. break; } + // Guard against underflow (blockSize < 4) making the offset negative + // and bypassing the loop bounds check, and against advancing past the + // end of the buffer. + if (blockSize < 4 || (u64)offset + blockSize - 4 > dataLength) { + return SCE_ERROR_ATRAC_SIZE_TOO_SMALL; + } offset += blockSize - 4; }