From 77133dd82c4f0c7e5a8742b0874e6f9ad57b0b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 11:37:35 +0200 Subject: [PATCH] Fix a long-standing buffer overflow in savedata encryption 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. --- Core/Dialog/SavedataParam.cpp | 2 +- Core/ELF/ParamSFO.cpp | 9 ++++----- Core/ELF/ParamSFO.h | 6 ++++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Core/Dialog/SavedataParam.cpp b/Core/Dialog/SavedataParam.cpp index fc944483ee..c9533f2f68 100644 --- a/Core/Dialog/SavedataParam.cpp +++ b/Core/Dialog/SavedataParam.cpp @@ -473,7 +473,7 @@ int SavedataParam::Save(SceUtilitySavedataParam* param, const std::string &saveD u8 *data_ = param->dataBuf; int aligned_len = align16(cryptedSize); - cryptedData = new u8[aligned_len + 0x10]; + cryptedData = new u8[aligned_len + 0x10]{}; memcpy(cryptedData, data_, cryptedSize); int decryptMode = DetermineCryptMode(param); diff --git a/Core/ELF/ParamSFO.cpp b/Core/ELF/ParamSFO.cpp index bbd14ca454..856221d36c 100644 --- a/Core/ELF/ParamSFO.cpp +++ b/Core/ELF/ParamSFO.cpp @@ -219,7 +219,7 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, const std::string &dataName) return -1; } -bool ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { +void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { size_t total_size = 0; size_t key_size = 0; size_t data_size = 0; @@ -251,9 +251,10 @@ bool ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { total_size += data_size; *size = total_size; - u8* data = new u8[total_size]; + size_t aligned_size = (total_size + 15) & ~15; + u8* data = new u8[aligned_size]; *paramsfo = data; - memset(data, 0, total_size); + memset(data, 0, aligned_size); memcpy(data, &header, sizeof(Header)); // Now fill @@ -300,8 +301,6 @@ bool ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { index_ptr++; } - - return true; } void ParamSFOData::Clear() { diff --git a/Core/ELF/ParamSFO.h b/Core/ELF/ParamSFO.h index 23af2f0c3f..3d7a719e50 100644 --- a/Core/ELF/ParamSFO.h +++ b/Core/ELF/ParamSFO.h @@ -42,9 +42,11 @@ public: std::string GetDiscID(); - bool ReadSFO(const u8 *paramsfo, size_t size); - bool WriteSFO(u8 **paramsfo, size_t *size) const; + // This allocates a buffer (*paramsfo) using new[], whose size is zero-filled up to a multiple of 16 bytes. + // This is required for SavedataParam::BuildHash. + void WriteSFO(u8 **paramsfo, size_t *size) const; + bool ReadSFO(const u8 *paramsfo, size_t size); bool ReadSFO(const std::vector ¶msfo) { if (!paramsfo.empty()) { return ReadSFO(¶msfo[0], paramsfo.size());