From 9f4dcf359b761bb981a90eb80e23b6b5b7bcc279 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 30 Aug 2026 22:45:50 +0200 Subject: [PATCH 1/3] Fix three more out-of-bounds writes GetCurrentDrawAsDebugVertices (GE debugger vertex preview) sized its index scratch buffer at a fixed 65536 and then ran both expanding steps into it: index generation turns strips/fans into up to 3 indices per input index, and RunSoftwareTransform can then expand points/lines/rects into 6 more each. A 30000-vertex triangle strip wrote ~90000 entries. Size the buffer from the count instead. The Expand{Rectangles,Lines,Points} capacity checks were also off: they compared the expansion against indsSize but write the expanded indices at inds + vertexCount, so the input count has to be part of the sum. ControlMapper::Axis wrote rawAxisValue_[axis.axisId] with no bounds check, one line below an explicit check on axis.deviceId. axisId comes straight from the device - Android reports AXIS_GENERIC_13..16 as 44..47, against a 44-entry array - so it wrote into the neighbouring deviceTimestamps_. NativeAxis had the same unchecked write into HLEPlugins::PluginDataAxis, where it goes out of the object entirely. Rewind's LockedDecompress computed its copy-from-base block size as base.size() - result.size() in size_t and truncated to int, so it went negative once the output grew past the base, and insert() then ran with last < first. That happens because a state can outlive the base it was compressed against: there are 20 states but only 2 bases, rotated every 16 saves. Track a generation per base and refuse to decode a state whose base is gone, and bound the block size against the base itself. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2 --- Core/ControlMapper.cpp | 5 ++++- Core/SaveStateRewind.cpp | 25 ++++++++++++++++++++++--- Core/SaveStateRewind.h | 4 ++++ GPU/Common/SoftwareTransformCommon.cpp | 15 ++++++++++----- UI/NativeApp.cpp | 5 ++++- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/Core/ControlMapper.cpp b/Core/ControlMapper.cpp index d939b3c64f..bb2a5dd748 100644 --- a/Core/ControlMapper.cpp +++ b/Core/ControlMapper.cpp @@ -683,7 +683,10 @@ void ControlMapper::Axis(const AxisInput *axes, size_t count) { if (deviceIndex < (size_t)DEVICE_ID_COUNT) { deviceTimestamps_[deviceIndex] = now; } - rawAxisValue_[axis.axisId] = axis.value; // these are only used for co-axis mapping + // Same as deviceId above, axisId comes straight from the device and can be out of range. + if ((size_t)axis.axisId < JOYSTICK_AXIS_MAX) { + rawAxisValue_[axis.axisId] = axis.value; // these are only used for co-axis mapping + } if (axis.value >= 0.0f) { InputMapping mapping(axis.deviceId, axis.axisId, 1); InputMapping opposite(axis.deviceId, axis.axisId, -1); diff --git a/Core/SaveStateRewind.cpp b/Core/SaveStateRewind.cpp index dc7eaf0c7a..d2b059d22e 100644 --- a/Core/SaveStateRewind.cpp +++ b/Core/SaveStateRewind.cpp @@ -29,6 +29,7 @@ CChunkFileReader::Error StateRingbuffer::Save() { { base_ = (base_ + 1) % ARRAY_SIZE(bases_); baseUsage_ = 0; + baseGeneration_[base_] = nextBaseGeneration_++; err = SaveToRam(bases_[base_]); // Let's not bother savestating twice. compressBuffer = &bases_[base_]; @@ -42,7 +43,7 @@ CChunkFileReader::Error StateRingbuffer::Save() { states_[n].clear(); } - baseMapping_[n] = base_; + baseMapping_[n] = baseGeneration_[base_]; return err; } @@ -59,8 +60,17 @@ CChunkFileReader::Error StateRingbuffer::Restore(std::string *errorString, std:: auto pa = GetI18NCategory(I18NCat::PAUSE); + const int generation = baseMapping_[n]; + const int baseSlot = generation < 0 ? -1 : generation % (int)ARRAY_SIZE(bases_); + if (baseSlot < 0 || baseGeneration_[baseSlot] != generation) { + // The base this state was compressed against has since been overwritten, so it can't be + // decoded any more. Only two bases are kept, but the state ring is longer. + WARN_LOG(Log::SaveState, "Rewind: state %d was compressed against a base that's gone", n); + return CChunkFileReader::ERROR_BAD_FILE; + } + static std::vector buffer; - LockedDecompress(buffer, states_[n].stateBuffer, bases_[baseMapping_[n]]); + LockedDecompress(buffer, states_[n].stateBuffer, bases_[baseSlot]); CChunkFileReader::Error error = LoadFromRam(buffer, errorString); *metadata = pa->T("Rewind"); @@ -117,7 +127,12 @@ void StateRingbuffer::LockedDecompress(std::vector &result, const std::vecto for (size_t i = 0; i < compressed.size(); ) { if (compressed[i] == 0) { ++i; - int blockSize = std::min(BLOCK_SIZE, (int)(base.size() - result.size())); + // Bound against what's actually left of the base: the subtraction this used to do + // (base.size() - result.size()) wraps once the output is longer than the base. + const int blockSize = (int)std::min((size_t)BLOCK_SIZE, (size_t)(base.end() - basePos)); + if (blockSize <= 0) { + break; + } result.insert(result.end(), basePos, basePos + blockSize); basePos += blockSize; } else { @@ -145,6 +160,10 @@ void StateRingbuffer::Clear() { for (auto &b : bases_) { b.clear(); } + for (int &g : baseGeneration_) { + g = -1; + } + nextBaseGeneration_ = 0; baseMapping_.clear(); baseMapping_.resize(size_); for (auto &s : states_) { diff --git a/Core/SaveStateRewind.h b/Core/SaveStateRewind.h index 16eddec118..da6a2c1afa 100644 --- a/Core/SaveStateRewind.h +++ b/Core/SaveStateRewind.h @@ -65,6 +65,10 @@ private: std::vector states_; StateBuffer bases_[2]; + // Which generation each base slot currently holds, and which generation each state was + // compressed against. There are more states than bases, so states do go stale. + int baseGeneration_[2] = {-1, -1}; + int nextBaseGeneration_ = 0; std::vector baseMapping_; std::mutex lock_; std::thread compressThread_; diff --git a/GPU/Common/SoftwareTransformCommon.cpp b/GPU/Common/SoftwareTransformCommon.cpp index e5fd954ad4..b8f5ff2c2a 100644 --- a/GPU/Common/SoftwareTransformCommon.cpp +++ b/GPU/Common/SoftwareTransformCommon.cpp @@ -955,7 +955,8 @@ static SoftwareTransformAction ProjectClipAndExpand(SoftwareTransformParams &par static bool ExpandRectangles(int vertexCount, int &numDecodedVerts, int vertsSize, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int *drawIndexCount, bool throughmode, bool *pixelMappedExactly) { // Before we start, do a sanity check - does the output fit? - if ((vertexCount / 2) * 6 > indsSize) { + // The expanded indices are written after the input ones, at inds + vertexCount. + if (vertexCount + (vertexCount / 2) * 6 > indsSize) { // Won't fit, kill the draw. return false; } @@ -1128,7 +1129,8 @@ void IndexBufferProvokingLastToFirst(int prim, u16 *inds, int indsSize) { static bool ExpandLines(int vertexCount, int &numDecodedVerts, int vertsSize, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int *drawIndexCount, bool throughmode) { // Before we start, do a sanity check - does the output fit? - if ((vertexCount / 2) * 6 > indsSize) { + // The expanded indices are written after the input ones, at inds + vertexCount. + if (vertexCount + (vertexCount / 2) * 6 > indsSize) { // Won't fit, kill the draw. return false; } @@ -1261,7 +1263,8 @@ static bool ExpandLines(int vertexCount, int &numDecodedVerts, int vertsSize, u1 static bool ExpandPoints(int vertexCount, int &maxIndex, int vertsSize, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int *drawIndexCount, bool throughmode, float pointScale) { // Before we start, do a sanity check - does the output fit? - if (vertexCount * 6 > indsSize) { + // The expanded indices are written after the input ones, at inds + vertexCount. + if (vertexCount + vertexCount * 6 > indsSize) { // Won't fit, kill the draw. return false; } @@ -1487,9 +1490,11 @@ bool GetCurrentDrawAsDebugVertices(DrawEngineCommon *drawEngine, GECommand cmd, const u32 vertTypeID = GetVertTypeID(gstate.vertType, gstate.getUVGenMode()); const bool throughMode = (vertTypeID & GE_VTYPE_THROUGH) != 0; - // Points is the only primitive that generates 6x as many vertices as input indices (2 triangles per point). + // Two expansions happen in here, and both multiply the input count, so a fixed 65536 wasn't + // enough: index generation turns strips/fans into up to 3 indices per input index, and then + // RunSoftwareTransform can expand points/lines/rects to 6 more each, written after those. std::vector indexTemp; - indexTemp.resize(65536); // (prim == GEPrimitiveType::GE_PRIM_POINTS ? count * 6 : count * 3) * 4); + indexTemp.resize((size_t)count * 3 * 7 + 32); // First, inspect the indices to find the range we need to decode. const u8 *indsPtr = Memory::GetPointerUnchecked(gstate_c.indexAddr); diff --git a/UI/NativeApp.cpp b/UI/NativeApp.cpp index 2b06c0104d..1c90474bc2 100644 --- a/UI/NativeApp.cpp +++ b/UI/NativeApp.cpp @@ -1730,7 +1730,10 @@ void NativeAxis(const AxisInput *axes, size_t count) { for (size_t i = 0; i < count; i++) { const AxisInput &axis = axes[i]; - HLEPlugins::PluginDataAxis[axis.axisId] = axis.value; + // axisId comes straight from the device, and can exceed the axes we know about. + if ((size_t)axis.axisId < JOYSTICK_AXIS_MAX) { + HLEPlugins::PluginDataAxis[axis.axisId] = axis.value; + } } } From 14111097da539f23b2edf05bbaf472a703e18a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 31 Aug 2026 13:37:00 +0200 Subject: [PATCH 2/3] Claude found this code buggy, and it's not used, so delete it. --- Core/ConfigSettings.cpp | 23 ----------------------- Core/ConfigSettings.h | 2 -- 2 files changed, 25 deletions(-) diff --git a/Core/ConfigSettings.cpp b/Core/ConfigSettings.cpp index 97776658d2..9a866b1ea7 100644 --- a/Core/ConfigSettings.cpp +++ b/Core/ConfigSettings.cpp @@ -371,29 +371,6 @@ bool ConfigSetting::RestoreToDefault(ConfigBlock *configBlock, bool log) const { return false; } -// Might be used to copy individual settings from defaulted blocks. Didn't end up using this for now. -void ConfigSetting::CopyFromBlock(const ConfigBlock *other) { - _dbg_assert_(offset_ >= 0 && offset_ < other->Size()); - - const char *otherOwner = (const char *)other; - const char *thisOwner = (const char *)this; - switch (type_) { - case Type::TYPE_BOOL: *(bool *)(thisOwner + offset_) = *(const bool *)(otherOwner + offset_); break; - case Type::TYPE_INT: *(int *)(thisOwner + offset_) = *(const int *)(otherOwner + offset_); break; - case Type::TYPE_UINT32: *(uint32_t *)(thisOwner + offset_) = *(const uint32_t *)(otherOwner + offset_); break; - case Type::TYPE_UINT64: *(uint64_t *)(thisOwner + offset_) = *(const uint64_t *)(otherOwner + offset_); break; - case Type::TYPE_FLOAT: *(float *)(thisOwner + offset_) = *(const float *)(otherOwner + offset_); break; - case Type::TYPE_STRING: *(std::string *)(thisOwner + offset_) = *(const std::string *)(otherOwner + offset_); break; - case Type::TYPE_STRING_VECTOR: *(std::vector *)(thisOwner + offset_) = *(const std::vector *)(otherOwner + offset_); break; - case Type::TYPE_PATH: *(Path *)(thisOwner + offset_) = *(const Path *)(otherOwner + offset_); break; - case Type::TYPE_TOUCH_POS: *(ConfigTouchPos *)(thisOwner + offset_) = *(const ConfigTouchPos *)(otherOwner + offset_); break; - case Type::TYPE_CUSTOM_BUTTON: *(ConfigCustomButton *)(thisOwner + offset_) = *(const ConfigCustomButton *)(otherOwner + offset_); break; - default: - _dbg_assert_msg_(false, "CopyFromBlock(%.*s): Unexpected setting type: %d", STR_VIEW(iniKey_), (int)type_); - return; - } -} - void ConfigSetting::ReportSetting(const ConfigBlock *configBlock, UrlEncoder &data, const std::string &prefix) const { if (!Report()) return; diff --git a/Core/ConfigSettings.h b/Core/ConfigSettings.h index 04472b4fd0..81d311f42d 100644 --- a/Core/ConfigSettings.h +++ b/Core/ConfigSettings.h @@ -190,8 +190,6 @@ struct ConfigSetting { bool ReadFromIniSection(ConfigBlock *configBlock, const Section *section, bool applyDefaultIfMissing) const; - void CopyFromBlock(const ConfigBlock *other); - // Yes, this can be const because what's modified is not the ConfigSetting struct, but the value which is stored elsewhere. // Should actually be called WriteToIni or something. void WriteToIniSection(const ConfigBlock *configBlock, Section *section) const; From a4082bede63f9e09f5e08b0c69aa44085c629e68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 31 Aug 2026 16:16:49 +0200 Subject: [PATCH 3/3] Bump gradle to 9.7.1 --- gradle/wrapper/gradle-wrapper.properties | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index a9db11550c..28e470264d 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,9 +1,6 @@ +#Mon Aug 31 16:15:56 CEST 2026 distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-9.6.1-bin.zip -networkTimeout=10000 -retries=0 -retryBackOffMs=500 -validateDistributionUrl=true +distributionUrl=https\://services.gradle.org/distributions/gradle-9.7.1-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists