Improve gamedata validation

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<uint8_t*>(pAddress);
+
+		for (size_t i = 0; i < vecVerify.size(); i++) {
+			// wildcard '*' skip
+			if (vecVerify[i] == static_cast<uint8_t>('*')) {
+				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();
 }
 

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<uint8_t>(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;
 }
 

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;