The "is this an ELF rather than a PBP" test compared against "\nFLE", which is
neither ELF's magic (\x7fELF) nor anything else - most likely a \x7f escape that
swallowed the E when it was written in 2013. Since no real file matches it,
every file that wasn't a PBP was reported as an ELF and the error branch was
unreachable. Compares against the real magic now, so something that's neither is
reported as neither. That error also printed the 4-byte magic with %s, which
isn't NUL-terminated - it's four hex bytes instead.
GetSubFileSize subtracted offsets that come straight out of the file without
checking they're ordered or even inside it, so a corrupt PBP produced a size
from an unsigned underflow - nearly 4GB, which the callers then had to catch by
size limit. It returns 0 for anything that doesn't make sense.
Also &(*out)[0] on a zero-length subfile, which is UB on an empty vector.
Plus one in ParamSFO: GetDataOffset mixed int and u32 for the data offset, so
its bounds check ran in whichever type the promotion landed on. It's size_t
throughout now, matching how ReadSFO does the same arithmetic.
Booted an EBOOT.PBP to check the PBP path end to end - loads, and generates the
same fake disc ID as before. pspautotests 314/314, UnitTest 55/55.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
The fake disc ID homebrew gets when it has no PARAM.SFO is built from the sum of
the bytes of its folder name, summed through a plain char - which is signed on
x86 and unsigned on ARM. So the same homebrew folder produced one ID on Windows
and a different one on Android, quietly splitting its savestates and per-game
config between platforms. Sum through unsigned char, which is what the ARM
builds (Android, iOS, Apple Silicon) already did.
Uppercasing is now explicit and ASCII-only rather than toupper(). Passing a
negative char to toupper() is undefined and trips MSVC's debug CRT assert, so a
folder with a non-ASCII name could stop a debug build dead, and what it did with
bytes above 0x7F otherwise depended on the locale.
ASCII folder names - very nearly all of them - produce exactly the same ID as
before. Non-ASCII ones change on the signed-char platforms, to what the
unsigned-char ones were already generating.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
Each entry gets param_max_len bytes in the data table, and that's what the
buffer is sized from - but nothing clamped what got written into it.
For VT_UTF8 the length written was s_value.size()+1, and then a terminator was
stored at data_ptr[param_len], one byte beyond that again. The memcpy already
copies the terminator (param_len counts it), so that store was both redundant
and always out of range. It doesn't take a malformed file to hit: several
callers pass the string's own length as max_size - see PSPLoaders.cpp's TITLE,
DISC_ID and DISC_VERSION - so the entry overran by two bytes every time, and a
128-character SAVEDATA_TITLE in a 128-byte slot wrote its terminator into the
next entry's data. VT_UTF8_SPE had the same missing clamp without the
off-by-one.
Both are clamped now and log when they truncate, and a negative max_size no
longer subtracts from the computed buffer size. Bytes written are unchanged for
values that do fit, which is every normal case - the terminator now comes from
the zero-fill instead of an explicit store - so this doesn't change any savedata
the emulator produces.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
ReadSFO dereferenced index table entries without checking the table fit
within the buffer, and GetDataOffset had no bounds checks at all (reading
attacker-controlled offsets and strcmp'ing without a terminator guard).
- Validate the index table fits entirely within the buffer in ReadSFO.
- Add a size parameter to GetDataOffset and validate the index table,
key/data table positions, and key string termination before use.
* Rename LogType to Log
* Explicitly use the Log:: enum when logging. Allows for autocomplete when editing.
* Mac/ARM64 buildfix
* Do the same with the hle result log macros
* Rename the log names to mixed case while at it.
* iOS buildfix
* Qt buildfix attempt, ARM32 buildfix
BuildHash pads the buffer up to its 16-byte aligned size with zeroes,
so there needs to be space for that. Or, we should just remove that
write, but let's do the smallest change that fixes the bug for now.
a-blondel