From f4010bea0f36fb20a2d7ebbe6017f6d5b45918e8 Mon Sep 17 00:00:00 2001 From: A1mDev <33463136+A1mDev@users.noreply.github.com> Date: Fri, 3 Oct 2025 00:06:35 +0700 Subject: [PATCH] Improve gamedata validation - Added stricter processing of game configuration data. - Previously, the extension often returned no errors, even if some sections were incorrect, giving a false sense of successful patch application. - Now, malformed sections or invalid entries trigger warnings or errors, making it clear when something is wrong. - This helps prevent silent failures and speeds up development and testing by alerting users to typos or misconfigurations in the patch data. - Fixed some type cast warnings. -Ensured proper initialization of MemoryPatchInfo fields, avoiding uninitialized offset values. --- types/mempatch.cpp | 16 +++++----- userconf/mempatches.cpp | 67 ++++++++++++++++++++++++++++++++++++----- userconf/mempatches.h | 7 +++++ 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/types/mempatch.cpp b/types/mempatch.cpp index 1b62ec1..5e7c5f7 100644 --- a/types/mempatch.cpp +++ b/types/mempatch.cpp @@ -55,7 +55,6 @@ class MemoryPatch { SH_MEM_READ | SH_MEM_WRITE | SH_MEM_EXEC); ByteVectorWrite(vecPatch, (uint8_t*) pAddress); - // for (size_t i = 0; i < vecPatch.size(); i++) { uint8_t preserveBits = 0; if (i < vecPreserve.size()) { @@ -81,15 +80,20 @@ class MemoryPatch { return false; } - auto addr = (uint8_t*) pAddress; - for (size_t i = 0; i < this->vecVerify.size(); i++) { - if (vecVerify[i] != '*' && vecVerify[i] != addr[i]) { + auto addr = reinterpret_cast(pAddress); + + for (size_t i = 0; i < vecVerify.size(); i++) { + // wildcard '*' skip + if (vecVerify[i] == static_cast('*')) { + continue; + } + + if (vecVerify[i] != addr[i]) { return false; } } return true; } - ~MemoryPatch() { this->Disable(); } @@ -127,7 +131,6 @@ cell_t sm_MemoryPatchLoadFromConfig(IPluginContext *pContext, const cell_t *para if (!pConfig) { return pContext->ThrowNativeError("Invalid game config handle %x (error %d)", hndl, err); } - void* addr; if (!pConfig->GetMemSig(info.signature.c_str(), &addr)) { return pContext->ThrowNativeError("Failed to locate signature for '%s' (mempatch '%s')", info.signature.c_str(), name); @@ -159,7 +162,6 @@ cell_t sm_MemoryPatchEnable(IPluginContext *pContext, const cell_t *params) { if ((err = ReadMemoryPatchHandle(hndl, &pMemoryPatch)) != HandleError_None) { return pContext->ThrowNativeError("Invalid MemoryPatch handle %x (error %d)", hndl, err); } - return pMemoryPatch->Enable(); } diff --git a/userconf/mempatches.cpp b/userconf/mempatches.cpp index a9d2d2a..1a2a451 100644 --- a/userconf/mempatches.cpp +++ b/userconf/mempatches.cpp @@ -24,7 +24,7 @@ ByteVector ByteVectorFromString(const char* s) { char* s1 = strdup(s); char* p = strtok(s1, "\\x "); while (p) { - uint8_t byte = strtol(p, nullptr, 16); + uint8_t byte = static_cast(strtol(p, nullptr, 16)); payload.push_back(byte); p = strtok(nullptr, "\\x "); } @@ -141,26 +141,79 @@ SMCResult MemPatchGameConfig::ReadSMC_KeyValue(const SMCStates *states, const ch { case PState_Runtime: if (!strcmp(key, "signature")) { - g_CurrentPatchInfo->signature = value; + if (value && value[0] != '\0') { + g_CurrentPatchInfo->signature = value; + } else { + smutils->LogError(myself, "Error: empty signature in patch \"%s\" at line %i col %i", \ + g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; + } } else if (!strcmp(key, "offset")) { - // Support for IDA-style address offsets - if (value[strlen(value)-1] == 'h') { - g_CurrentPatchInfo->offset = (size_t) strtol(value, nullptr, 16); + // size_t - supports only unsigned int + std::string s(value); + char* end = nullptr; + + if (s.empty()) { + smutils->LogError(myself, "Error: empty offset in patch \"%s\" at line %i col %i", \ + g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; } else { - g_CurrentPatchInfo->offset = atoi(value); + if (s.back() == 'h' || s.back() == 'H') { + // IDA-style hex: "10h" + s.pop_back(); + g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 16); + } else if (s.rfind("0x", 0) == 0 || s.rfind("0X", 0) == 0) { + // C-style hex: "0x10" + g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 16); + } else { + // Decimal + g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 10); + } + + if (*end != '\0') { + smutils->LogError(myself, "Error: Invalid offset value \"%s\" at line %i col %i",\ + value, states->line, states->col); + return SMCResult_HaltFail; + } } } else if (!strcmp(key, "patch")) { g_CurrentPatchInfo->vecPatch = ByteVectorFromString(value); + + if (g_CurrentPatchInfo->vecPatch.size() < 1) { + smutils->LogError(myself, "Error: empty patch section in patch \"%s\" at line %i col %i", + g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; + } } else if (!strcmp(key, "verify")) { g_CurrentPatchInfo->vecVerify = ByteVectorFromString(value); + + if (g_CurrentPatchInfo->vecVerify.size() < 1) { + smutils->LogError(myself, "Error: empty verify section in patch \"%s\" at line %i col %i", + g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; + } } else if (!strcmp(key, "preserve")) { g_CurrentPatchInfo->vecPreserve = ByteVectorFromString(value); + + if (g_CurrentPatchInfo->vecPreserve.size() < 1) { + smutils->LogError(myself, "Error: empty preserve section in patch \"%s\" at line %i col %i", + g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; + } + } else { + // Force users to check their gamedata in case something is incorrect. + // It's possible that the user made a typo, and the extension doesn't display errors; + // This has been verified in practice. + smutils->LogError(myself, "Error: unknown key \"%s\" in patch \"%s\" at line %i col %i", + key, g_CurrentSection.c_str(), states->line, states->col); + return SMCResult_HaltFail; } break; default: - smutils->LogError(myself, "Unknown key in MemPatches section \"%s\": line: %i col: %i", key, states->line, states->col); + smutils->LogError(myself, "Error: Unknown key in MemPatches section \"%s\": line: %i col: %i", key, states->line, states->col); return SMCResult_HaltFail; } + return SMCResult_Continue; } diff --git a/userconf/mempatches.h b/userconf/mempatches.h index ad27737..cee1e4a 100644 --- a/userconf/mempatches.h +++ b/userconf/mempatches.h @@ -20,6 +20,13 @@ class MemPatchGameConfig : public ITextListener_SMC { public: class MemoryPatchInfo { public: + MemoryPatchInfo() + { + // Other class variables are themselves initialized with a default value, + // but this one is not, it contains a garbage value + offset = 0; + } + std::string signature; size_t offset; ByteVector vecPatch, vecVerify, vecPreserve;