Skip to content

Commit 815d2ff

Browse files
A1mDevnosoop
authored andcommitted
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.
1 parent d48a173 commit 815d2ff

3 files changed

Lines changed: 76 additions & 14 deletions

File tree

types/mempatch.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ class MemoryPatch {
5555
SH_MEM_READ | SH_MEM_WRITE | SH_MEM_EXEC);
5656
ByteVectorWrite(vecPatch, (uint8_t*) pAddress);
5757

58-
//
5958
for (size_t i = 0; i < vecPatch.size(); i++) {
6059
uint8_t preserveBits = 0;
6160
if (i < vecPreserve.size()) {
@@ -81,15 +80,20 @@ class MemoryPatch {
8180
return false;
8281
}
8382

84-
auto addr = (uint8_t*) pAddress;
85-
for (size_t i = 0; i < this->vecVerify.size(); i++) {
86-
if (vecVerify[i] != '*' && vecVerify[i] != addr[i]) {
83+
auto addr = reinterpret_cast<uint8_t*>(pAddress);
84+
85+
for (size_t i = 0; i < vecVerify.size(); i++) {
86+
// wildcard '*' skip
87+
if (vecVerify[i] == static_cast<uint8_t>('*')) {
88+
continue;
89+
}
90+
91+
if (vecVerify[i] != addr[i]) {
8792
return false;
8893
}
8994
}
9095
return true;
9196
}
92-
9397
~MemoryPatch() {
9498
this->Disable();
9599
}
@@ -127,7 +131,6 @@ cell_t sm_MemoryPatchLoadFromConfig(IPluginContext *pContext, const cell_t *para
127131
if (!pConfig) {
128132
return pContext->ThrowNativeError("Invalid game config handle %x (error %d)", hndl, err);
129133
}
130-
131134
void* addr;
132135
if (!pConfig->GetMemSig(info.signature.c_str(), &addr)) {
133136
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) {
159162
if ((err = ReadMemoryPatchHandle(hndl, &pMemoryPatch)) != HandleError_None) {
160163
return pContext->ThrowNativeError("Invalid MemoryPatch handle %x (error %d)", hndl, err);
161164
}
162-
163165
return pMemoryPatch->Enable();
164166
}
165167

userconf/mempatches.cpp

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ ByteVector ByteVectorFromString(const char* s) {
2424
char* s1 = strdup(s);
2525
char* p = strtok(s1, "\\x ");
2626
while (p) {
27-
uint8_t byte = strtol(p, nullptr, 16);
27+
uint8_t byte = static_cast<uint8_t>(strtol(p, nullptr, 16));
2828
payload.push_back(byte);
2929
p = strtok(nullptr, "\\x ");
3030
}
@@ -141,26 +141,79 @@ SMCResult MemPatchGameConfig::ReadSMC_KeyValue(const SMCStates *states, const ch
141141
{
142142
case PState_Runtime:
143143
if (!strcmp(key, "signature")) {
144-
g_CurrentPatchInfo->signature = value;
144+
if (value && value[0] != '\0') {
145+
g_CurrentPatchInfo->signature = value;
146+
} else {
147+
smutils->LogError(myself, "Error: empty signature in patch \"%s\" at line %i col %i", \
148+
g_CurrentSection.c_str(), states->line, states->col);
149+
return SMCResult_HaltFail;
150+
}
145151
} else if (!strcmp(key, "offset")) {
146-
// Support for IDA-style address offsets
147-
if (value[strlen(value)-1] == 'h') {
148-
g_CurrentPatchInfo->offset = (size_t) strtol(value, nullptr, 16);
152+
// size_t - supports only unsigned int
153+
std::string s(value);
154+
char* end = nullptr;
155+
156+
if (s.empty()) {
157+
smutils->LogError(myself, "Error: empty offset in patch \"%s\" at line %i col %i", \
158+
g_CurrentSection.c_str(), states->line, states->col);
159+
return SMCResult_HaltFail;
149160
} else {
150-
g_CurrentPatchInfo->offset = atoi(value);
161+
if (s.back() == 'h' || s.back() == 'H') {
162+
// IDA-style hex: "10h"
163+
s.pop_back();
164+
g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 16);
165+
} else if (s.rfind("0x", 0) == 0 || s.rfind("0X", 0) == 0) {
166+
// C-style hex: "0x10"
167+
g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 16);
168+
} else {
169+
// Decimal
170+
g_CurrentPatchInfo->offset = strtoul(s.c_str(), &end, 10);
171+
}
172+
173+
if (*end != '\0') {
174+
smutils->LogError(myself, "Error: Invalid offset value \"%s\" at line %i col %i",\
175+
value, states->line, states->col);
176+
return SMCResult_HaltFail;
177+
}
151178
}
152179
} else if (!strcmp(key, "patch")) {
153180
g_CurrentPatchInfo->vecPatch = ByteVectorFromString(value);
181+
182+
if (g_CurrentPatchInfo->vecPatch.size() < 1) {
183+
smutils->LogError(myself, "Error: empty patch section in patch \"%s\" at line %i col %i",
184+
g_CurrentSection.c_str(), states->line, states->col);
185+
return SMCResult_HaltFail;
186+
}
154187
} else if (!strcmp(key, "verify")) {
155188
g_CurrentPatchInfo->vecVerify = ByteVectorFromString(value);
189+
190+
if (g_CurrentPatchInfo->vecVerify.size() < 1) {
191+
smutils->LogError(myself, "Error: empty verify section in patch \"%s\" at line %i col %i",
192+
g_CurrentSection.c_str(), states->line, states->col);
193+
return SMCResult_HaltFail;
194+
}
156195
} else if (!strcmp(key, "preserve")) {
157196
g_CurrentPatchInfo->vecPreserve = ByteVectorFromString(value);
197+
198+
if (g_CurrentPatchInfo->vecPreserve.size() < 1) {
199+
smutils->LogError(myself, "Error: empty preserve section in patch \"%s\" at line %i col %i",
200+
g_CurrentSection.c_str(), states->line, states->col);
201+
return SMCResult_HaltFail;
202+
}
203+
} else {
204+
// Force users to check their gamedata in case something is incorrect.
205+
// It's possible that the user made a typo, and the extension doesn't display errors;
206+
// This has been verified in practice.
207+
smutils->LogError(myself, "Error: unknown key \"%s\" in patch \"%s\" at line %i col %i",
208+
key, g_CurrentSection.c_str(), states->line, states->col);
209+
return SMCResult_HaltFail;
158210
}
159211
break;
160212
default:
161-
smutils->LogError(myself, "Unknown key in MemPatches section \"%s\": line: %i col: %i", key, states->line, states->col);
213+
smutils->LogError(myself, "Error: Unknown key in MemPatches section \"%s\": line: %i col: %i", key, states->line, states->col);
162214
return SMCResult_HaltFail;
163215
}
216+
164217
return SMCResult_Continue;
165218
}
166219

userconf/mempatches.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ class MemPatchGameConfig : public ITextListener_SMC {
2020
public:
2121
class MemoryPatchInfo {
2222
public:
23+
MemoryPatchInfo()
24+
{
25+
// Other class variables are themselves initialized with a default value,
26+
// but this one is not, it contains a garbage value
27+
offset = 0;
28+
}
29+
2330
std::string signature;
2431
size_t offset;
2532
ByteVector vecPatch, vecVerify, vecPreserve;

0 commit comments

Comments
 (0)