patchmanager: fix signature patches reporting neither on or off (#873)
## Link to GitHub Issue or related Pull Request, if one exists
None
## Description of change
Patch Manager showed valid signature patches as broken: **"Bad patch;
patch is neither on or off"** instead of Enabled/Disabled, so they could
not be toggled.
`SignaturePatch::to_memory` had two bugs. In the configurator it cached
a file offset as `data_offset_ptr`, so status checks `memcmp`'d a fake
address. It also passed JSON `offset` into `find_pattern` while still
indexing the signature/replacement from 0, which mis-aligned every patch
with `offset != 0`. This change locates the signature start, applies
`offset` afterward, compares only the replacement window, and leaves the
pointer null so `is_patch_active` re-resolves from `data_offset`.
Not proposed as built-in patches. The JSON below is the reproduction
case: each entry uses `offset > 0` and a replacement shorter than the
signature.
## Testing
Reproduced in Patch Manager against `bm2dx.dll` using the signature JSON
below. Before the fix, every patch reported "neither on or off". After
the fix, each patch locates, shows Disabled/Enabled, and toggling writes
only the replacement bytes at `signature_match + offset`.
## Demo
<details>
<summary>Signature JSON used to reproduce (offset + short
replacement)</summary>
```json
[
{
"info": "streaming / getcm patches (type=signature)",
"gameCode": "LDJ",
"notes": "Each site uses a unique signature (usage=0 only)."
},
{
"type": "group",
"id": "streaming-getcm",
"name": "Streaming getcm",
"description": "Enable all children so streaming.common merges without Banner FS and getcm can fire without visiting Test Mode.",
"gameCode": "LDJ"
},
{
"name": "Streaming: merge common without Banner",
"description": "NOP jz in streaming.common callback so CM work table is filled even when Banner FS is still null.",
"caution": "Required. Without this, early common responses are discarded and getcm stays empty.",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "E8????????4885C00F84????????488D0D????????488D15????????41B848080000",
"replacement": "909090909090",
"offset": 8,
"usage": 0
},
{
"name": "Streaming: scheduler without Banner (common)",
"description": "NOP jz after Banner getter on the common branch of the periodic scheduler.",
"caution": "Enable with the getcm scheduler sibling. Unique via imul of common-interval dword.",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "E8????????4885C0742469057E791C0AE8030000",
"replacement": "9090",
"offset": 8,
"usage": 0
},
{
"name": "Streaming: scheduler without Banner (getcm)",
"description": "NOP jz after Banner getter on the getcm branch of the periodic scheduler.",
"caution": "Enable with the common scheduler sibling. Unique via imul of getcm-interval dword. Without this, getcm never schedules while Banner FS is null.",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "E8????????4885C07424690574B91D0AE8030000",
"replacement": "9090",
"offset": 8,
"usage": 0
},
{
"name": "Streaming: fall into getcm after common",
"description": "NOP jmp-after-common so the same scheduler tick can evaluate getcm instead of returning early.",
"caution": "Pair with Banner scheduler skips (or a live Banner FS).",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "891D????????E9????????E8????????4885C074246905",
"replacement": "9090909090",
"offset": 6,
"usage": 0
},
{
"name": "Streaming: getcm interval 1s #1",
"description": "Default getcm poll interval 1800s to 1s (first init store).",
"caution": "Enable #1 and #2 together. Trailing BF3C000000 distinguishes this init site.",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "C705????????18150000C705????????08070000C705????????201C0000BF3C000000",
"replacement": "01000000",
"offset": 16,
"usage": 0
},
{
"name": "Streaming: getcm interval 1s #2",
"description": "Default getcm poll interval 1800s to 1s (second init store).",
"caution": "Enable #1 and #2 together. Trailing 448925 distinguishes this init site.",
"gameCode": "LDJ",
"type": "signature",
"group": "streaming-getcm",
"dllName": "bm2dx.dll",
"signature": "C705????????18150000C705????????08070000C705????????201C0000448925",
"replacement": "01000000",
"offset": 16,
"usage": 0
}
]
```
</details>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -669,6 +669,11 @@ namespace patcher {
|
|||||||
signature.erase(std::remove(signature.begin(), signature.end(), ' '), signature.end());
|
signature.erase(std::remove(signature.begin(), signature.end(), ' '), signature.end());
|
||||||
replacement.erase(std::remove(replacement.begin(), replacement.end(), ' '), replacement.end());
|
replacement.erase(std::remove(replacement.begin(), replacement.end(), ' '), replacement.end());
|
||||||
|
|
||||||
|
if (signature.empty() || (signature.length() % 2) != 0
|
||||||
|
|| replacement.empty() || (replacement.length() % 2) != 0) {
|
||||||
|
return {.fatal_error = true};
|
||||||
|
}
|
||||||
|
|
||||||
// build pattern
|
// build pattern
|
||||||
std::string pattern_str(signature);
|
std::string pattern_str(signature);
|
||||||
strreplace(pattern_str, "??", "00");
|
strreplace(pattern_str, "??", "00");
|
||||||
@@ -717,13 +722,26 @@ namespace patcher {
|
|||||||
}
|
}
|
||||||
std::string replace_mask_str = replace_mask.str();
|
std::string replace_mask_str = replace_mask.str();
|
||||||
|
|
||||||
// find offset
|
// replacement applies at signature_match + offset; must stay inside the signature
|
||||||
|
const size_t sig_len = signature_mask_str.length();
|
||||||
|
const size_t repl_len = replace_mask_str.length();
|
||||||
|
if (offset > sig_len || repl_len == 0 || offset + repl_len > sig_len) {
|
||||||
|
log_warning("patchmanager",
|
||||||
|
"signature patch '{}': offset {} + replacement {} exceeds signature {}",
|
||||||
|
patch->name, offset, repl_len, sig_len);
|
||||||
|
return {.fatal_error = true};
|
||||||
|
}
|
||||||
|
|
||||||
|
// Locate signature *start* (find_pattern offset arg = 0). JSON "offset" is applied
|
||||||
|
// below. Old code passed JSON offset into find_pattern but still indexed
|
||||||
|
// signature/replacement from 0 → mis-aligned when offset != 0.
|
||||||
uint64_t data_offset = 0;
|
uint64_t data_offset = 0;
|
||||||
uint8_t *data_offset_ptr = nullptr;
|
uint8_t *read_ptr = nullptr;
|
||||||
uintptr_t data_offset_ptr_base = 0;
|
HMODULE module = nullptr;
|
||||||
|
bool module_free = false;
|
||||||
|
|
||||||
if (cfg::CONFIGURATOR_STANDALONE) {
|
if (cfg::CONFIGURATOR_STANDALONE) {
|
||||||
|
|
||||||
// load file into dll map if missing
|
|
||||||
auto it = DLL_MAP.find(dll_name);
|
auto it = DLL_MAP.find(dll_name);
|
||||||
if (it == DLL_MAP.end()) {
|
if (it == DLL_MAP.end()) {
|
||||||
DLL_MAP[dll_name] =
|
DLL_MAP[dll_name] =
|
||||||
@@ -732,16 +750,21 @@ namespace patcher {
|
|||||||
it = DLL_MAP.find(dll_name);
|
it = DLL_MAP.find(dll_name);
|
||||||
}
|
}
|
||||||
|
|
||||||
// find pattern
|
// base=0 → file offset of signature start; 0 also means "not found"
|
||||||
data_offset = find_pattern(*it->second, 0, pattern_bin.get(), signature_mask_str.c_str(), offset, usage);
|
const intptr_t match = find_pattern(
|
||||||
data_offset_ptr = reinterpret_cast<uint8_t *>(data_offset);
|
*it->second, 0, pattern_bin.get(), signature_mask_str.c_str(), 0, usage);
|
||||||
data_offset_ptr_base = (uintptr_t) it->second->data();
|
if (match == 0) {
|
||||||
|
return {.fatal_error = true};
|
||||||
|
}
|
||||||
|
data_offset = static_cast<uint64_t>(match) + offset;
|
||||||
|
if (data_offset + repl_len > it->second->size()) {
|
||||||
|
return {.fatal_error = true};
|
||||||
|
}
|
||||||
|
read_ptr = it->second->data() + data_offset;
|
||||||
|
|
||||||
} else {
|
} else {
|
||||||
|
|
||||||
// get module
|
module = libutils::try_module(dll_path);
|
||||||
auto module = libutils::try_module(dll_path);
|
|
||||||
bool module_free = false;
|
|
||||||
if (!module) {
|
if (!module) {
|
||||||
module = libutils::try_library(dll_path);
|
module = libutils::try_library(dll_path);
|
||||||
if (module) {
|
if (module) {
|
||||||
@@ -751,59 +774,71 @@ namespace patcher {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// find pattern
|
const intptr_t match_va = find_pattern(
|
||||||
data_offset_ptr = reinterpret_cast<uint8_t *>(
|
module, pattern_bin.get(), signature_mask_str.c_str(), 0, usage);
|
||||||
find_pattern(module, pattern_bin.get(), signature_mask_str.c_str(), offset, usage));
|
auto *match_ptr = reinterpret_cast<uint8_t *>(match_va);
|
||||||
|
if (match_ptr == nullptr) {
|
||||||
// convert back to offset
|
|
||||||
data_offset = libutils::rva2offset(dll_path, (intptr_t) (data_offset_ptr - (uint8_t*) module));
|
|
||||||
|
|
||||||
// clean
|
|
||||||
if (module_free) {
|
if (module_free) {
|
||||||
FreeLibrary(module);
|
FreeLibrary(module);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// check pointers
|
|
||||||
if (data_offset_ptr == nullptr) {
|
|
||||||
return {.fatal_error = true};
|
return {.fatal_error = true};
|
||||||
}
|
}
|
||||||
|
|
||||||
// get disabled/enabled data
|
const intptr_t file_off = libutils::rva2offset(
|
||||||
size_t data_len = std::max(signature_mask_str.length(), replace_mask_str.length());
|
dll_path, (intptr_t) (match_ptr - (uint8_t *) module));
|
||||||
std::shared_ptr<uint8_t[]> data_disabled(new uint8_t[data_len]);
|
if (file_off < 0) {
|
||||||
std::shared_ptr<uint8_t[]> data_enabled(new uint8_t[data_len]);
|
if (module_free) {
|
||||||
memutils::VProtectGuard data_guard(data_offset_ptr + data_offset_ptr_base, data_len);
|
FreeLibrary(module);
|
||||||
for (size_t i = 0; i < data_len; ++i) {
|
}
|
||||||
if (i >= signature_mask_str.length() || signature_mask_str[i] != 'X') {
|
return {.fatal_error = true};
|
||||||
data_disabled.get()[i] = (data_offset_ptr + data_offset_ptr_base)[i];
|
}
|
||||||
|
data_offset = static_cast<uint64_t>(file_off) + offset;
|
||||||
|
read_ptr = match_ptr + offset;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build disabled/enabled for the replacement window only.
|
||||||
|
std::shared_ptr<uint8_t[]> data_disabled(new uint8_t[repl_len]);
|
||||||
|
std::shared_ptr<uint8_t[]> data_enabled(new uint8_t[repl_len]);
|
||||||
|
{
|
||||||
|
memutils::VProtectGuard data_guard(read_ptr, repl_len);
|
||||||
|
for (size_t i = 0; i < repl_len; ++i) {
|
||||||
|
const size_t si = static_cast<size_t>(offset) + i;
|
||||||
|
if (signature_mask_str[si] != 'X') {
|
||||||
|
data_disabled.get()[i] = read_ptr[i];
|
||||||
} else {
|
} else {
|
||||||
data_disabled.get()[i] = pattern_bin.get()[i];
|
data_disabled.get()[i] = pattern_bin.get()[si];
|
||||||
}
|
}
|
||||||
}
|
if (replace_mask_str[i] != 'X') {
|
||||||
for (size_t i = 0; i < data_len; ++i) {
|
data_enabled.get()[i] = read_ptr[i];
|
||||||
if (i >= replace_mask_str.length() || replace_mask_str[i] != 'X') {
|
|
||||||
data_enabled.get()[i] = (data_offset_ptr + data_offset_ptr_base)[i];
|
|
||||||
} else {
|
} else {
|
||||||
data_enabled.get()[i] = replace_data_bin.get()[i];
|
data_enabled.get()[i] = replace_data_bin.get()[i];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Drop temporary LoadLibrary mapping only after reads above.
|
||||||
|
if (module_free) {
|
||||||
|
FreeLibrary(module);
|
||||||
|
}
|
||||||
|
|
||||||
// log edit
|
|
||||||
log_misc("patchmanager", "found {}: {:#08X}: {} -> {}",
|
log_misc("patchmanager", "found {}: {:#08X}: {} -> {}",
|
||||||
patch->name, data_offset,
|
patch->name, data_offset,
|
||||||
bin2hex(data_disabled.get(), data_len),
|
bin2hex(data_disabled.get(), repl_len),
|
||||||
bin2hex(data_enabled.get(), data_len));
|
bin2hex(data_enabled.get(), repl_len));
|
||||||
|
|
||||||
// build patch
|
// BUGFIX: never cache a pointer here.
|
||||||
|
// - standalone used to store (uint8_t*)file_offset, so is_patch_active skipped
|
||||||
|
// re-resolve and memcmp'd a fake address → "neither on or off".
|
||||||
|
// - in-game used to keep a pointer that FreeLibrary may invalidate.
|
||||||
|
// is_patch_active / apply_patch re-resolve from data_offset when ptr is nullptr.
|
||||||
return MemoryPatch {
|
return MemoryPatch {
|
||||||
.dll_name = dll_name,
|
.dll_name = dll_name,
|
||||||
.data_disabled = std::move(data_disabled),
|
.data_disabled = std::move(data_disabled),
|
||||||
.data_disabled_len = data_len,
|
.data_disabled_len = repl_len,
|
||||||
.data_enabled = std::move(data_enabled),
|
.data_enabled = std::move(data_enabled),
|
||||||
.data_enabled_len = data_len,
|
.data_enabled_len = repl_len,
|
||||||
.data_offset = data_offset,
|
.data_offset = data_offset,
|
||||||
.data_offset_ptr = data_offset_ptr,
|
.data_offset_ptr = nullptr,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user