HookingSystem::Detach dereferences a possibly-null item.original

[JustinMDotNet]

Summary

HookingSystem::Detach unconditionally writes *item.original = nullptr after a successful detach. The v1 API entry point does not require plugins to provide a non-null aOriginal, and the matching Attach path correctly guards the write with if (aOriginal). So a plugin that registered a hook without asking for the original trampoline will crash the game process when it later removes that hook.

Where

• src/dll/v1/Funcs.cpp:36-60 — v1::Hooking::Attach only null-checks aTarget and aDetour; aOriginal is allowed to be null.
• src/dll/Systems/HookingSystem.cpp:46-77 — Attach guards the write: if (aOriginal) *aOriginal = ...;.
• src/dll/Systems/HookingSystem.cpp:116-128 — Detach does the write unconditionally:

for (auto it = range.first; it != range.second;)
{
    auto& item = it->second;
    if (item.target == aTarget)
    {
        *item.original = nullptr;   // <-- AV if item.original is null
        it = m_hooks.erase(it);
    }
    else
    {
        ++it;
    }
}

HookingSystem::Shutdown() is not affected because it uses QueueForDetach + m_hooks.clear() and never writes through item.original.

Reproduction

A plugin does:

sdk->hooking->Attach(handle, target, detour, nullptr);
// ... later ...
sdk->hooking->Detach(handle, target);

Access violation in the game process at the *item.original = nullptr; line.

Suggested fix

Guard the write the same way Attach does:

if (item.original)
{
    *item.original = nullptr;
}
it = m_hooks.erase(it);

(Alternative: reject aOriginal == nullptr at the v1 API boundary in Funcs.cpp. The guard is the safer choice because it preserves the current optional-out semantics.)

Severity

High. Deterministic crash from a valid API usage pattern. One-line fix.