Get module entry by name from external program via WinAPI

Question

I'm asking myself if there's any better/shorter way of getting a module entry by its name from an external process.

This is the code I have so far:

MODULEENTRY32 GetModule(const char* ModuleName) {

    HANDLE Module = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, pid);
    MODULEENTRY32 Entry;
    Entry.dwSize = sizeof(Entry);

    WCHAR *ModuleNameChar;
    int Chars = MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, NULL, 0);
    ModuleNameChar = new WCHAR[Chars];
    MultiByteToWideChar(CP_ACP, 0, ModuleName, -1, (LPWSTR)ModuleNameChar, Chars);

    while (Module32Next(Module, &Entry)) {
        if (!wcscmp((wchar_t*)Entry.szModule, ModuleNameChar)) {
            CloseHandle(Module);
            return Entry;
        }
    }

    /*
        Return the module base 0x0 if we don't find any module
    */
    CloseHandle(Module);
    Entry.modBaseAddr = 0x0;
    return Entry;
}

It should work but it's quite complicated. Is there any useful function or method I can make use of to simplify this whole thing?

Answer 1

Unfortunately, I'm not sure of any way of being more concise.

There are a few changes you could make, but they're mostly optional.

1. Check the return value of CreateToolhelp32Snapshot(). The documentation says the following:

   Includes all modules of the process specified in th32ProcessID in the snapshot. To enumerate the modules, see Module32First. If the function fails with ERROR_BAD_LENGTH, retry the function until it succeeds.

   I use CreateToolhelp32Snapshot() in an application that runs 24/7 and I know for a fact that I get the ERROR_BAD_LENGTH error here and there. In my case, I cannot afford false negatives, so I have to put this function in a loop. If you are okay with having false-negatives occasionally, then I wouldn't worry too much about it.

2. I don't know if you're showing us the full code for this function, but you have a memory leak here:

   ModuleNameChar = new WCHAR[Chars];

   I don't see where you are deleting this buffer. I personally would use an std::vector<WCHAR> as a buffer, give it the correct size, and pass the address of the first element to MultiByteToWideChar().

3. If you use HANDLEs a lot, consider wrapping them in an RAII container. That way, you don't have to worry about closing them manually. This will also make your code exception-safe if you ever add any functions that can throw (you actually already have one that can throw, the new call).

4. I noticed you're not calling ::Module32First(). You probably don't need it, but I always have it anyway.

5. This is a personal preference, but I think your upper-case variable names look absolutely horrendous. Please use snake-case or camel-case for variable names.

There's really nothing helpful to say unfortunately, the Windows API can be quite cumbersome.

Answer 2

more simply api, way not exist.

1.) wrong return type - MODULEENTRY32. really impossible return structure, which not fit generic register size. when compiler view T fn(..) { * } it silently change this to void fn(T* t, ...) {*} and in your code Entry will be copied, what really not need. you must not try return MODULEENTRY32 but pass pointer to it as function argument.

2.) you lost error value. the GetModule can fail by different reasons - can not exist process with such id (STATUS_INVALID_CID), you can fail open target process (STATUS_ACCESS_DENIED), module can simply not exist in process. without returned error value - you can not know reason why function fail. really the best practice always return win32 error code (or NTSTATUS) from function.

3.) why you use module name as ansi string ? this is always bad and error design. use unicode name format. and you will be not need convert it (i already not say that if module name (this is rarely and never for system modules) use non english chars - it can not have correct ansi name at all(and depend from system locale - you encode it on self comp in one locale, on another comp - can be another locale. but ansi name depend from it).

4.) so signature of function must be ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)

5.) use wcscmp is wrong here. we need case insensitive compare. - _wcsicmp

6.) Return the module base 0x0 if we don't find any module - how i say - we need return appropriate error code in this case, instead set base address at 0 (we can left it undefined). client code must check not base address for 0, but error code for 0 (NOERROR).

7.) not forget about 32-64 bit issues here (are your and target process have some bit-ness)

so code can look like:

ULONG GetModule(DWORD th32ProcessID, MODULEENTRY32* Entry, PCWSTR szModule)
{
    HANDLE hSnapshot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, th32ProcessID);

    if (hSnapshot != INVALID_HANDLE_VALUE)
    {
        ULONG dwError = ERROR_NOT_FOUND;

        Entry->dwSize = sizeof(MODULEENTRY32);

        if (Module32FirstW(hSnapshot, Entry))
        {
            do 
            {
                if (!_wcsicmp(Entry->szModule, szModule))
                {
                    dwError = NOERROR;
                    break;
                }
            } while (Module32NextW(hSnapshot, Entry));
        }

        CloseHandle(hSnapshot);

        return dwError;
    }

    // return RtlGetLastNtStatus(); 
    // this is much more better compare GetLastError();
    return GetLastError();
}

and usage:

MODULEENTRY32 Entry;
if (ULONG dwError = GetModule(*, &Entry, L"*"))
{
    // error, dont touch Entry
}
else
{
    //ok, use Entry
}