preserve-emulation/ppsspp
1
0
Fork 0
mirror of https://github.com/hrydgard/ppsspp.git synced 2026-09-03 19:25:18 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add savestate serializer tests, fix three bounds-check bugs

Browse Source 
PointerWrap and the Do() overloads around it are how every savestate is
written and read, and had no direct coverage. Everything read back came off
disk, so the corrupt-input paths matter as much as the round trips.

Three bugs, all in the bounds checking added in 58d4759ceb:

1. sizeof(T) is not a lower bound on how many bytes an element serializes to.
   It only holds for the types DoHelper_ writes out raw. A std::string is 32-40
   bytes in memory and serializes to as few as five; a T* serializes to whatever
   T::DoState() writes. So DoVector/DoList/DoSet/DoMap could reject a perfectly
   valid savestate whenever count * sizeof(element) exceeded the bytes left in
   the buffer. That is not hypothetical: pspFileSystem is serialized dead last
   in SaveStart::DoState, and MetaFileSystem::DoState does Do(p, currentDir) on
   a std::map<int, std::string>, so the check runs with only a few hundred bytes
   remaining and claims 44 bytes per entry against roughly 22 actual. Added
   SerializeMinElemSize<T>(), mirroring DoHelper_'s own condition, and used it
   in all five containers. The bound is only loosened, so nothing that loaded
   before can stop loading.

2. Do(p, std::map<K, T *> &) deletes every value before reading the new ones,
   and DoMap then returned on a bad count without clearing - leaving the map
   full of freed pointers to be used or deleted again. Six live maps go through
   this (sceMpeg, sceMp3, sceAac, sceFont, sceHeap, sceKernelThread's pending
   calls), so a corrupt savestate meant a use-after-free. Clear before the guard
   can bail out, in DoMap, DoMultimap and DoSet.

3. The wstring and u16string overloads validated stringLen < 0 but not 0, and
   didn't require a whole number of characters. read() computes
   stringLen / sizeof(char) - 1, so a length of 0 resized to SIZE_MAX and
   memcpy'd with a wrapped-around size. PSPOskDialog::DoState serializes both
   (inputChars at v2, a legacy wstring below that), so this was reachable: the
   test aborts the process without the fix.

The test covers round trips of PODs, strings (empty, embedded NUL), vector,
map, set, list and map-of-pointers, section titles and version gating in both
directions, marker mismatches, measure-vs-write checkpoint disagreement, the
error latch dropping to MODE_NOOP, every truncation of a valid buffer, and
hand-corrupted counts and lengths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9
This commit is contained in:
Henrik Rydgård
2026-08-17 14:50:57 +02:00
co-authored by Claude Opus 5
parent 8265044a79
commit b42a03e095
6 changed files with 437 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Common/Serialize/Serializer.cpp
+6 -2
View File
@@ -217,7 +217,9 @@ void Do(PointerWrap &p, std::wstring &x) {
int stringLen = sizeof(wchar_t) * ((int)x.length() + 1);
Do(p, stringLen);
if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) {
// The length is in bytes, so it has to be a whole number of characters, and at least the NUL
// terminator. Otherwise read() below computes a negative character count.
if (stringLen < (int)sizeof(wchar_t) || (stringLen % sizeof(wchar_t)) != 0 || stringLen > MAX_SANE_STRING_LENGTH) {
WARN_LOG(Log::SaveState, "Savestate failure: bad stringLen %d", stringLen);
p.SetError(PointerWrap::ERROR_FAILURE);
return;
@@ -250,7 +252,9 @@ void Do(PointerWrap &p, std::u16string &x) {
int stringLen = sizeof(char16_t) * ((int)x.length() + 1);
Do(p, stringLen);
if (stringLen < 0 || stringLen > MAX_SANE_STRING_LENGTH) {
// The length is in bytes, so it has to be a whole number of characters, and at least the NUL
// terminator. Otherwise read() below computes a negative character count.
if (stringLen < (int)sizeof(char16_t) || (stringLen % sizeof(char16_t)) != 0 || stringLen > MAX_SANE_STRING_LENGTH) {
WARN_LOG(Log::SaveState, "Savestate failure: bad stringLen %d", stringLen);
p.SetError(PointerWrap::ERROR_FAILURE);
return;