blocksActuallyRead rounded bytesRead up to the next whole block
unconditionally, intending to handle the legitimate case where the
very last block of the file is naturally shorter than BLOCK_SIZE
(cache_ is deliberately over-allocated for that). But it applied the
same rounding to any short read, including a genuine failure or a
dropped connection mid-file (this loader can sit on top of the whole
Remote ISO chain via CachingFileLoader/HTTPFileLoader when "Cache full
ISO in RAM" is enabled) - marking a block as fully cached when only a
few of its bytes were actually written. Since cache_ is malloc'd (not
zeroed), every later read of that block would serve uninitialized heap
memory as if it were real file data.
Only round up when the short read's end position exactly matches the
true end of the file.
SaveIntoCache checked `readBytes != 0` instead of comparing against the
full expected length, so any nonzero-but-short read from the backend
(e.g. a Remote ISO connection dropping mid-file) was treated as a
complete success: in the multi-block path this marked *all* requested
blocks (up to 16) as fully cached and wrote the uninitialized tail of
the read buffer to the on-disk cache file, and in both paths the
short/uninitialized data was also copied straight into the caller's
output buffer and counted in the return value - so a read failure was
reported (and permanently cached) as success. Only treat a block as
read once the backend actually delivered the full blockSize_ for it,
and stop before caching or returning anything for blocks it didn't.
Also fixes two latent bugs in the same functions, unreachable in the
current call graph (DiskCachingFileLoader is only ever driven by
CachingFileLoader, which always issues block-aligned reads) but wrong
if ever called otherwise:
- The multi-block loop reused the batch's initial `offset` (the
position within the *first* block) for every subsequent block
instead of resetting it to 0, which would both read from the wrong
place in `wholeRead` and mis-copy less than a full block for i > 0.
- ReadBlockData() applied `offset` to the destination pointer instead
of the file seek position, which would both read the wrong bytes
from disk and write up to `offset` bytes past the end of the
caller's buffer.
LoadCacheIndex's sanity check on persisted block indices used `>`
instead of `>=` against maxBlocks_ (blockIndexLookup_ only has
maxBlocks_ entries, valid indices 0..maxBlocks_-1), so a corrupted
cache file's index entry with block == maxBlocks_ exactly would pass
validation and then index one past the end of blockIndexLookup_.
SaveIntoCache() discarded backend_->ReadAt()'s return value entirely
and unconditionally marked the requested block(s) as cached. A short
or failed read from the backend (e.g. a Remote ISO connection dropping
mid-file, now that LocalFileLoader/RetryingFileLoader correctly report
failures as 0 rather than a huge count) would still get stored as a
"valid" cached block, permanently serving its uninitialized tail as if
it were real file data on every later read, with no retry.
Only insert a block once we've confirmed the backend actually
delivered the full BLOCK_SIZE for it.
ReadAt()'s contract is to return the number of bytes/units actually
read. On every platform branch, an OS-level read failure (ReadFile
returning FALSE, or pread/read returning -1) was fed straight into a
division by `bytes` without checking for it first:
- Windows explicitly returned (size_t)-1.
- Elsewhere, the signed -1 from pread/read was implicitly converted to
size_t (via the usual arithmetic conversions with the unsigned
`bytes`) before the division, producing a huge bogus count instead
of a small one.
Every caller in the caching chain (CachingFileLoader,
RamCachingFileLoader, RetryingFileLoader, ZipFileLoader's libzip
source callback) loops on "did we get at least what we asked for",
which a huge return value trivially satisfies - so a local I/O error
(removable media ejected, a content-URI permission problem mid-read,
etc.) would be reported as a fully successful read of whatever
uninitialized memory happened to be in the destination buffer.
sceFontGetCharGlyphImage_Clip stored the glyph over the destination buffer.
Hardware adds it with saturation instead, so on hardware a glyph never erases
what was already in the buffer - the transparent parts contribute zero - while
we cleared them to 0. Text drawn as overlapping glyphs lost the earlier ones.
Measured on a PSP-1000 against a dumped ltn0.pgf, sweeping the pen position's
fractional part, both 4bpp and 8bpp, and several destination pre-fill values so
that "wrote a 0 here" could be told apart from "did not write here":
- The result is min(dst + blend, max), not blend.
- The two horizontal weights are rounded in opposite directions, down for the
left neighbour and up for the pixel itself. That is not a free choice: it is
what makes the pair sum to exactly the format's full scale whenever both
samples are full ink, for every fraction. Rounding the combined sum once, as
before, disagrees with hardware on about a fifth of the blended pixels.
- 4bpp blends the raw nibble and 8bpp the value swizzled to 8 bit. Blending at
8 bit and narrowing afterwards is not the same thing and misses ~0.6%.
- The fractional part of yPos64 is discarded entirely. There is no vertical
blending and the rectangle never grows downwards, so renderY2 loses its +1
and the two render loops collapse into one - xFrac == 0 needs no special
case, the first term vanishes and the second collapses to exactly b.
Only PSP_FONT_PIXELFORMAT_4 and _8 render at all. _4_REV, _24 and _32 return 0
and leave the buffer completely untouched, which is also the answer to the
"not sure how to make these produce an image" note in pspautotests'
charglyphimageclip.cpp. They are skipped now, with a report so that a game
relying on them can be found - only one buffer configuration was measured.
Verified with pspautotests: the new charglyphimagexfrac goes from 388 of 601
differing lines to passing, and charglyphimage and charglyphimageclip improve
from 57 differing lines to 3. No other font test changes. The remaining 3 are
a separate, pre-existing issue with bytesPerLine of 0 and 1.
Not covered: clip rectangles that actually clip, negative pen positions, and
the shadow glyph entry points.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Henrik Rydgård