UriDecode() formed SRC_END - 2 unconditionally, a pointer before the
start of the buffer (UB) for a 0- or 1-byte input. IsLocalAbsolutePath()
indexed path[0]/path[1] on a std::string_view with no bounds check,
UB for an empty path (path[0]) or a 1-byte path on Windows (path[1]).
Neither was known to crash in practice, but both are real UB flagged
by hardened/UBSan builds and easy to trigger (e.g. an empty query
string, or listing the VFS root).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
ReadFrame() accepted a 64-bit client-supplied payload length with only
a top-bit check, and ReadPending() immediately resized a buffer by it
before any data had arrived - a single frame claiming a huge length
(reachable via the WebSocket debugger endpoint) could trigger a
multi-exabyte allocation attempt. Now rejected up front (both the
single frame and the fragmented-message total) against a 64MB cap.
Also replaced &payload[0]/&vector[0] with .data() in the send/receive
paths - operator[] on a possibly-empty vector (e.g. an empty PING) is
UB even when the result is never dereferenced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
ReadFile()/ReadSingleFileFromZip() allocated/resized directly off a
zip entry's declared uncompressed size with no sanity check. A
crafted size near UINT64_MAX would wrap ReadFile()'s "size + 1" to 0,
allocating almost nothing while zip_fread() still writes the full
declared size into it - a length-field-driven heap overflow from a
malicious zip/texture pack. Both now reject entries above a generous
4GB cap. Also fixes GetFileInfo() reading zstat.name[strlen(name)-1]
unchecked, which underflows to SIZE_MAX for a zero-length entry name.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
ParseHttpHeader() used strchr(buffer, ' ') unconditionally as endptr,
even though the parser explicitly supports HTTP/0.9-style requests
with no trailing space/version (type = SIMPLE). A request line like
"GET /" with no space made strchr return null, and nullptr - buffer
truncated to a garbage length driving new[]/memcpy. Falls back to the
end of the line when no space is found, and clamps param_length to
avoid a similar issue when '?' appears after the (missing) space.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
The fallback title was not shown on the game info screen when `PARAM.SFO` was missing.
Hoist the `TextView` creation from the `!regionID.empty()` branch.
`PSP_DISC_DIRECTORY` was the only `IdentifiedFileType` with possible `PARAM.SFO` that didn't mark `PARAM_SFO` ready inside the switch,
causing `GetSaveDataDirectories` to bail out and return an empty vector and the game info UI to show no savedata size info.
GrabMemSpace() ignored a NULL return from CreateFileMapping and
returned true anyway, unlike every other platform's arena backend
(Darwin, Android, Posix all check and return false on failure). Under
memory pressure this let startup proceed to CreateView()/MapViewOfFileEx
with an invalid handle instead of failing cleanly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
Update()/Entries()/ClickEntry()/Show()/SetProgressBar()/RemoveProgressBar()
locked mutex_ around entries_, but CancelById/ShowAchievementUnlocked/
ShowAchievementProgress/ShowChallengeIndicator/ShowLeaderboardTracker/
ClearAchievementStuff/SetClickCallback/SetFlags didn't. Core/WebServer.cpp
runs its own thread and calls SetClickCallback() while the main thread's
Update() concurrently erases/iterates the same vector every frame.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
DoVector already rejects an attacker/corruption-controlled size that
would resize far beyond what's actually left in the savestate buffer.
DoList/DoDeque/DoMap/DoMultimap/DoSet never got the same treatment -
a corrupted count field (e.g. 0xFFFFFFFF) drove an immediate huge
resize (list/deque) or an unbounded loop of allocations (map/set)
before any per-element bounds checking kicked in. All five now check
the declared count against PointerWrap::Remaining() first.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
MODE_READ built the string via x = (char*)*p.ptr, which strlen()s for
a NUL terminator. CheckRead(stringLen) only guarantees stringLen bytes
are available in the buffer, not that one of them is a NUL - a
corrupted savestate missing the terminator caused strlen to scan past
the checked region. Now uses a length-bounded assign(), matching how
the wstring/u16string siblings already do this correctly via memcpy.
Also tightens the stringLen validity check to require >= 1 (matching
what a real serialized string always has), so stringLen - 1 can't
go negative for a corrupted stringLen of 0.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
When vsnprintf's return value (the would-have-been length) was >=
sizeof(buffer), the code logged a truncation warning but then still
memcpy'd that full, untruncated length out of the 4096-byte stack
buffer, reading past its end. retval is now clamped to what vsnprintf
actually wrote before use.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
initPostfixExpression() filled a fixed char subStr[256] with no bound
check while tokenizing numeric literals and identifiers. A 256+ char
token in a debugger expression (breakpoint condition, watch,
memory.search) - settable over the network via the WebSocket debugger
- smashed the stack. Now bails out with an error once the token
reaches the buffer size.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
VR_DestroyRenderer() freed projections without nulling it, and
VR_GetView() indexed it with no initialized check - a latent
use-after-free if VR_GetView were ever called between a destroy and
the next VR_InitRenderer. Not currently reachable (VR_DestroyRenderer
is only called from inside VR_InitRenderer, synchronously followed by
reallocation), but cheap to close off.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
INVR_Vibrate()'s channel selection was `i & chan` (bitwise AND) inside
a loop over i in [0,2), instead of just using chan directly as the
index. For chan=0 (left controller, per the only call site iterating
j in [0,2)), i & 0 is always 0, so the "if (channel)" check was never
true and vibration_channel_duration/intensity were never set - the
left controller silently never vibrated. chan=1 (right) happened to
work by coincidence (i=1 gives 1 & 1 == 1). chan is now used directly
as the array index, with no loop needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
FromConfigString() indexed parts[0]/parts[1] from SplitString() with
no check that at least 2 parts were produced - a malformed/truncated
line in a hand-edited controls.ini (no '-') was an OOB vector access.
Separately, DEVICE_ID_ANY (-1) didn't round-trip correctly:
ToConfigString() formats it as e.g. "-1-5", but splitting that on '-'
produces "", "1", "5" instead of "-1", "5" - deviceId decoded to 0 and
keyCode to 1 instead of 5. Rather than special-casing the negative
sign to make it round-trip, just reject DEVICE_ID_ANY mappings
outright - it's not something we want to support, and the whole ANY
concept is likely going away. Preserves the existing (tested) behavior
of tolerating a MultiInputMapping string and parsing just its first
mapping, via atoi()'s stop-at-first-non-digit behavior.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
The found binary path could be relative (e.g. 'PPSSPPHeadless' from the
repo root in CI), but tests run with the output dir as cwd, so launching
failed with FileNotFoundError. Resolve the path against the script's cwd,
and turn launch failures into per-test ERROR results instead of crashing
the whole run.
AGENTS.md: never push without asking first.
The only guard against writing past the fixed 65536-entry verts_
array was _dbg_assert_msg_, which is compiled out entirely outside
_DEBUG - a large enough single batch (e.g. Circle()/CircleSegment()
with a big segment count, or a large UI list without an intervening
Flush()) silently corrupted the heap in release builds. Added a real
bounds check that drops the vertex instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
TextureLoadTask::Run() only freed the VFS-read file buffer on the
success path - a corrupt/truncated image in a texture pack or
replacement texture directory (valid magic, bad payload) leaked the
whole file's bytes on every load attempt.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
The test set lives in the 'frametests' git submodule (hrydgard/ppsspp-frametest-ci).
The test job previously only inited pspautotests, so the submodule was never
synced on CI and the (now removed) hashFiles gates skipped the frametests steps.
Use an HTTPS submodule URL so CI can fetch it without SSH keys.
The memmove length was computed from size_ after ExtendByOne() had
already bumped it, so it moved (oldSize - pos + 1) elements instead of
(oldSize - pos) - reading one uninitialized element past the old data
and writing one element past the new logical size. Currently masked
by ExtendByOne()'s growth policy always leaving capacity slack, but
a real overflow waiting for that assumption to not hold. Now captures
the old size before extending.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
next_crlf_offset() called peek() one byte past the currently buffered
data whenever a '\r' was the very last byte received (a normal TCP
fragmentation boundary) - peek() has no way to signal "not enough
data yet" and just asserts. Now checks there's actually a next byte
before peeking.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
next() would read a truncated lead byte's "second byte" unconditionally,
even when that byte was actually the string's null terminator - leaving
index_ one past the terminator, so a subsequent end()/next() call read
one byte out of bounds. Now checks for the terminator before consuming
it, returning INVALID without advancing past it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
RIFFReader::ReadData() trusted its count argument completely and
memcpy'd straight from the internal buffer with no bounds check.
Hardened it to clamp against the buffer and zero-fill any shortfall,
as defense in depth.
The actual reachable bug was in BackgroundAudio.cpp: it read a WAV
'smpl' chunk into a vector sized by GetCurrentChunkSize(), then
unconditionally indexed smplData[28] (and, for the loop array,
smplData[36]) with no check that the chunk was actually that large -
a short/corrupt chunk in a game's background-music WAV caused a heap
OOB read. Also fixes &smplData[0] being UB when the chunk is empty.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
LoadZIMPtr() writes width[]/height[]/image[] as arrays (one entry per
mip level, up to ZIM_MAX_MIP_LEVELS) whenever the file has
ZIM_HAS_MIPS set, per its documented contract - but this caller passed
plain scalar locals. A texture-replacement .zim file with that flag
set caused multiple out-of-bounds stack writes. Now passes properly
sized arrays and only uses level 0, matching the existing "we don't
support ZIM mips yet" behavior. Also fixes a pre-existing leak of
image[0] on the "changed since header read" error path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
These dereferenced get()'s result unconditionally, unlike the
two/three-arg "OrDefault" overloads which check. Hit on externally
sourced JSON: UI/Store.cpp reads the remote homebrew-store listing,
UI/DriverManagerScreen.cpp reads user-supplied GPU driver package
metadata - a field simply missing from either crashed the app instead
of failing gracefully.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L4QAoxV2KY7ek4PcZw3WvY
Henrik Rydgård