Savestate: Bounds-check sizes from the file, and plug leaks on load

Reject sizes past the end of the state before allocating (FPL, PGF,
achievements, SAS grain, savedata list, the memory fast path), fail
instead of desyncing on a SAS voice count mismatch, and free what old
states' paths and shrinking pointer containers dropped.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
Henrik RydgårdandClaude Opus 5.5 committed 2026-09-28 09:34:06 -06:00
1 parent 7fa6be6a25
commit 0a687b9435
12 files changed
+68 -3

No files matched your search

+8 -1
View File
@@ -27,7 +27,7 @@ void DoDeque(PointerWrap &p, std::deque<T> &x, T &default_val) {
Do(p, deq_size);
// Guard against an attacker-controlled size driving a huge resize, same as DoVector.
if (p.mode == PointerWrap::MODE_READ || p.mode == PointerWrap::MODE_VERIFY) {
if (deq_size > p.Remaining() / sizeof(T)) {
if (deq_size > p.Remaining() / SerializeMinElemSize<T>()) {
p.SetError(PointerWrap::ERROR_FAILURE);
return;
}
@@ -40,6 +40,13 @@ void DoDeque(PointerWrap &p, std::deque<T> &x, T &default_val) {
template<class T>
void Do(PointerWrap &p, std::deque<T *> &x) {
if (p.mode == PointerWrap::MODE_READ) {
// The elements are owned (DoClass replaces them), and a shorter deque would drop the rest.
for (T *elem : x) {
delete elem;
}
x.clear();
}
T *dv = nullptr;
DoDeque(p, x, dv);
}
+7
View File
@@ -126,6 +126,13 @@ void DoVector(PointerWrap &p, std::vector<T> &x, T &default_val) {
template<class T>
void Do(PointerWrap &p, std::vector<T *> &x) {
if (p.mode == PointerWrap::MODE_READ) {
// The elements are owned (DoClass replaces them), and a shorter vector would drop the rest.
for (T *elem : x) {
delete elem;
}
x.clear();
}
T *dv = nullptr;
DoVector(p, x, dv);
}
+7
View File
@@ -40,6 +40,13 @@ void DoList(PointerWrap &p, std::list<T> &x, T &default_val) {
template<class T>
void Do(PointerWrap &p, std::list<T *> &x) {
if (p.mode == PointerWrap::MODE_READ) {
// The elements are owned (DoClass replaces them), and a shorter list would drop the rest.
for (T *elem : x) {
delete elem;
}
x.clear();
}
T *dv = nullptr;
Do(p, x, dv);
}
+9
View File
@@ -1990,6 +1990,15 @@ void SavedataParam::DoState(PointerWrap &p) {
Do(p, saveNameListDataCount);
if (p.mode == p.MODE_READ) {
delete [] saveDataList;
saveDataList = nullptr;
if (saveDataListCount < 0 || !p.CheckRead(saveDataListCount)) {
saveDataListCount = 0;
saveNameListDataCount = 0;
p.SetError(p.ERROR_FAILURE);
return;
}
// Clear() leaves the name count behind, but it's only used to walk the list.
saveNameListDataCount = std::clamp(saveNameListDataCount, 0, saveDataListCount);
if (saveDataListCount != 0) {
saveDataList = new SaveFileInfo[saveDataListCount];
DoArray(p, saveDataList, saveDataListCount);
+5
View File
@@ -136,6 +136,11 @@ void PGF::DoState(PointerWrap &p) {
fontDataSize = (size_t)fontDataSizeTemp;
if (p.mode == p.MODE_READ) {
delete [] fontData;
fontData = nullptr;
if (!p.CheckRead(fontDataSize)) {
fontDataSize = 0;
return;
}
if (fontDataSize) {
fontData = new u8[fontDataSize];
DoArray(p, fontData, (int)fontDataSize);
+2 -1
View File
@@ -360,7 +360,8 @@ public:
if (s >= 3) {
Do(p, mode_);
} else {
mode_ = FONT_OPEN_INTERNAL_FULL;
// Only the destructor looks at this: a font loaded above is ours to delete.
mode_ = internalFont == -1 ? FONT_OPEN_USERBUFFER : FONT_OPEN_INTERNAL_FULL;
}
}
+5
View File
@@ -62,6 +62,11 @@ void __HeapDoState(PointerWrap &p) {
if (s >= 2) {
Do(p, heapList);
} else if (p.mode == p.MODE_READ) {
for (auto &[_, heap] : heapList) {
delete heap;
}
heapList.clear();
}
}
+6 -1
View File
@@ -105,8 +105,13 @@ void FPL::DoState(PointerWrap &p) {
return;
Do(p, nf);
if (p.mode == p.MODE_READ)
if (p.mode == p.MODE_READ) {
if (nf.numBlocks < 0 || !p.CheckRead(nf.numBlocks)) {
p.SetError(p.ERROR_FAILURE);
return;
}
blocks = new bool[nf.numBlocks];
}
DoArray(p, blocks, nf.numBlocks);
Do(p, address);
Do(p, alignedSize);
+6
View File
@@ -164,6 +164,11 @@ void __Mp3DoState(PointerWrap &p) {
if (s >= 2) {
Do(p, g_mp3Map);
} else {
for (auto &[_, mp3] : g_mp3Map) {
delete mp3;
}
g_mp3Map.clear();
std::map<u32, Mp3ContextOld *> mp3Map_old;
Do(p, mp3Map_old); // read old map
for (auto it = mp3Map_old.begin(), end = mp3Map_old.end(); it != end; ++it) {
@@ -188,6 +193,7 @@ void __Mp3DoState(PointerWrap &p) {
mp3->decoder = CreateAudioDecoder(PSP_CODEC_MP3);
g_mp3Map[id] = mp3;
delete mp3_old;
}
}
+6
View File
@@ -774,6 +774,11 @@ void SasInstance::DoState(PointerWrap &p) {
Do(p, grainSize);
if (p.mode == p.MODE_READ) {
if (grainSize > PSP_SAS_MAX_GRAIN) {
ERROR_LOG(Log::SaveState, "Bad SAS grain size %d", grainSize);
p.SetError(p.ERROR_FAILURE);
return;
}
if (grainSize > 0) {
SetGrainSize(grainSize);
} else {
@@ -803,6 +808,7 @@ void SasInstance::DoState(PointerWrap &p) {
Do(p, n);
if (n != PSP_SAS_VOICES_MAX) {
ERROR_LOG(Log::SaveState, "Wrong number of SAS voices");
p.SetError(p.ERROR_FAILURE);
return;
}
DoArray(p, voices, ARRAY_SIZE(voices));
+4
View File
@@ -369,6 +369,10 @@ static void DoMemoryVoid(PointerWrap &p, uint32_t start, uint32_t size) {
if ((size & 0x3F) != 0 || ((uintptr_t)d & 0x3F) != 0)
return p.DoVoid(d, size);
if ((p.mode == PointerWrap::MODE_READ || p.mode == PointerWrap::MODE_VERIFY) && !p.CheckRead(size)) {
return;
}
switch (p.mode) {
case PointerWrap::MODE_READ:
ParallelMemcpy(&g_threadManager, d, storage, size);
+3
View File
@@ -902,6 +902,9 @@ void DoState(PointerWrap &p) {
data_size = (uint32_t)(g_rcClient ? rc_client_progress_size(g_rcClient) : 0);
}
Do(p, data_size);
if (p.mode == PointerWrap::MODE_READ && !p.CheckRead(data_size)) {
return;
}
if (data_size > 0) {
uint8_t *buffer = new uint8_t[data_size];