Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions OpenKh.Research.Panacea/Panacea.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
#include "bass.h"
#include "bass_vgmstream.h"
#include <set>
#include <filesystem>
#include <Psapi.h>

bool ENSURE_DIRECTORY = false;

template <class TFunc>
class Hook
Expand Down Expand Up @@ -286,6 +290,29 @@ void LoadDLLs(const std::wstring& folder)
f(folder.c_str());
}

char* FetchPattern(const char* pattern, const char* patvalid)
{
HINSTANCE g_hInstance = GetModuleHandle(NULL);
MODULEINFO moduleInfo;
GetModuleInformation(GetCurrentProcess(), g_hInstance, &moduleInfo, sizeof(MODULEINFO));
const void* endAddress = (const char*)g_hInstance + moduleInfo.SizeOfImage;

for (char* addr = (char*)g_hInstance; addr < (char*)endAddress - 0x10; addr += 0x10)
{
size_t patlen = strlen(patvalid);

int i = 0;
for (; i < patlen; i++)
if (patvalid[i] != '?' && pattern[i] != addr[i])
break;

if (i == patlen)
return addr;
}

return nullptr;
}
Comment on lines 293 to 314
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

FetchPattern has undefined behavior: missing return when pattern is not found.

If no match is found, control falls off the end of a non-void function, which is UB in C++. The caller (line 1168/1172) will receive a garbage pointer and likely crash or silently corrupt state.

Additional concerns:

  1. Step size 0x10: Scanning in 16-byte increments will miss patterns at non-aligned offsets. While MSVC x64 tends to align functions to 16 bytes by default, this is not guaranteed for all build configurations — stepping by 1 is the safe choice for a general pattern scanner.
  2. Out-of-bounds read: The boundary addr < (char*)endAddress - 0x10 doesn't account for the actual pattern length (15 bytes). On the last valid iteration, addr[14] could read past the module image.
🐛 Proposed fix
 char* FetchPattern(const char* pattern, const char* patvalid)
 {
     HINSTANCE g_hInstance = GetModuleHandle(NULL);
     MODULEINFO moduleInfo;
     GetModuleInformation(GetCurrentProcess(), g_hInstance, &moduleInfo, sizeof(MODULEINFO));
     const void* endAddress = (const char*)g_hInstance + moduleInfo.SizeOfImage;
 
-    for (char* addr = (char*)g_hInstance; addr < (char*)endAddress - 0x10; addr += 0x10)
+    size_t patlen = strlen(patvalid);
+    for (char* addr = (char*)g_hInstance; addr + patlen <= (char*)endAddress; addr++)
     {
-        size_t patlen = strlen(patvalid);
-
         int i = 0;
         for (; i < patlen; i++)
             if (patvalid[i] != '?' && pattern[i] != addr[i])
                 break;
 
         if (i == patlen)
             return addr;
     }
+    return nullptr;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Research.Panacea/Panacea.cpp` around lines 293 - 312, FetchPattern
currently can fall off the end without returning (undefined behavior) and may
read out-of-bounds or skip matches by stepping 0x10; fix by computing size_t
patlen = strlen(patvalid) before the scan, set the scan step to 1 instead of
0x10, compute a safe last-start address using endAddress - patlen (or endAddress
- patlen + 1 as appropriate) to avoid reading past the module image, and ensure
the function returns nullptr when no match is found; keep references to the
existing symbols FetchPattern, pattern, patvalid, endAddress and
moduleInfo.SizeOfImage when making these changes.


void Panacea::Initialize()
{
ULONG_PTR baseImage = (ULONG_PTR)GetModuleHandle(nullptr);
Expand Down Expand Up @@ -1137,6 +1164,33 @@ long __cdecl Panacea::LoadFileWithSize(Axa::CFileMan* _this, const char* filenam

void* __cdecl Panacea::LoadFileWithMalloc(Axa::CFileMan* _this, const char* filename, int* sizePtr, bool useHdAsset, const char* filename2)
{
if (!ENSURE_DIRECTORY)
{
char _stringBuffer[256];
bool(*_fetchFunc)(const char* str, char* buff, char* a3) = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x60\x01\x00\x00", "xxxxxxxxxxxxxxx"));

if (!_fetchFunc)
{
_fetchFunc = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x40\x01\x00\x00", "xxxxxxxxxxxxxxx"));

if (!_fetchFunc)
{
ENSURE_DIRECTORY = true;
goto CONTINUE_FUNC;
}

_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Epic Games Store", _stringBuffer, nullptr);
}

else
_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Steam", _stringBuffer, nullptr);

std::filesystem::create_directories(_stringBuffer);
ENSURE_DIRECTORY = true;
}
Comment on lines 1167 to 1190
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Multiple crash-paths in the directory-ensuring block.

  1. Null function pointer dereference (line 1173): If both FetchPattern calls return nullptr (once the missing-return UB in FetchPattern is fixed), _fetchFunc is still null when line 1173 calls it → crash. There is no null guard before the second call.

  2. Uninitialized buffer passed to create_directories (line 1178): _stringBuffer is stack-allocated and never zero-initialized. If neither pattern resolves (or the resolved function doesn't write to the buffer), create_directories operates on garbage data.

  3. create_directories is called unconditionally (line 1178): Even when no valid path was resolved, the code proceeds to create directories. This should be guarded.

  4. Blank line between } and else (lines 1175-1176): While syntactically valid, the dangling else style is unusual and can be misleading during maintenance.

🐛 Proposed fix
     if (!ENSURE_DIRECTORY)
     {
-        char _stringBuffer[256];
+        char _stringBuffer[256] = {};
         bool(*_fetchFunc)(const char* str, char* buff, char* a3) = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x60\x01\x00\x00", "xxxxxxxxxxxxxxx"));
 
         if (!_fetchFunc)
         {
             _fetchFunc = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x40\x01\x00\x00", "xxxxxxxxxxxxxxx"));
-            _fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Epic Games Store", _stringBuffer, nullptr);
         }
-        
-        else
+
+        if (_fetchFunc)
+        {
+            // Try EGS first; fall back to Steam.
+            if (!_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Epic Games Store", _stringBuffer, nullptr))
+                _fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Steam", _stringBuffer, nullptr);
+
+            if (_stringBuffer[0])
+                std::filesystem::create_directories(_stringBuffer);
+        }
-            _fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Steam", _stringBuffer, nullptr);
-        std::filesystem::create_directories(_stringBuffer);
 
         ENSURE_DIRECTORY = true;
     }

Note: The proposed fix above changes the store-detection logic. The original code uses the first pattern to identify Steam vs. EGS, but the two FetchPattern calls appear to target different function signatures (differing only in stack frame size 0x60 vs 0x40). Please verify the intended semantics: which pattern corresponds to which store, and whether trying the EGS string first in both cases is correct for your use-case.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!ENSURE_DIRECTORY)
{
char _stringBuffer[256];
bool(*_fetchFunc)(const char* str, char* buff, char* a3) = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x60\x01\x00\x00", "xxxxxxxxxxxxxxx"));
if (!_fetchFunc)
{
_fetchFunc = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x40\x01\x00\x00", "xxxxxxxxxxxxxxx"));
_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Epic Games Store", _stringBuffer, nullptr);
}
else
_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Steam", _stringBuffer, nullptr);
std::filesystem::create_directories(_stringBuffer);
ENSURE_DIRECTORY = true;
}
if (!ENSURE_DIRECTORY)
{
char _stringBuffer[256] = {};
bool(*_fetchFunc)(const char* str, char* buff, char* a3) = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x60\x01\x00\x00", "xxxxxxxxxxxxxxx"));
if (!_fetchFunc)
{
_fetchFunc = reinterpret_cast<bool(*)(const char*, char*, char*)>(FetchPattern("\x48\x89\x5C\x24\x20\x55\x56\x57\x48\x81\xEC\x40\x01\x00\x00", "xxxxxxxxxxxxxxx"));
}
if (_fetchFunc)
{
// Try EGS first; fall back to Steam.
if (!_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Epic Games Store", _stringBuffer, nullptr))
_fetchFunc("KINGDOM HEARTS HD 1.5+2.5 ReMIX\\Steam", _stringBuffer, nullptr);
if (_stringBuffer[0])
std::filesystem::create_directories(_stringBuffer);
}
ENSURE_DIRECTORY = true;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@OpenKh.Research.Panacea/Panacea.cpp` around lines 1165 - 1181, The
directory-ensuring block can crash because _fetchFunc may be null, _stringBuffer
is uninitialized, and create_directories is unguarded; fix by (1) trying both
FetchPattern calls but checking the returned pointer before invoking it (use
FetchPattern(...)->assign to _fetchFunc and if still null skip directory logic),
(2) initialize or replace the stack buffer (char _stringBuffer[256]) with a
zeroed buffer or better a std::string/std::array to ensure a valid C-string, (3)
only call std::filesystem::create_directories when the resolved path string is
non-empty/valid, and (4) restructure the if/else so the fetch->call mapping is
explicit (e.g., if first pattern found use Steam string, else if second pattern
found use EGS string), and only set ENSURE_DIRECTORY = true after successful
directory creation; references: ENSURE_DIRECTORY, _fetchFunc, FetchPattern(...),
_stringBuffer, std::filesystem::create_directories.


CONTINUE_FUNC:

wchar_t path[MAX_PATH];
if (GetRawFile(path, sizeof(path), filename))
{
Expand Down
Loading