Trouble getting ModuleBaseAddress

[Amrok]

Hi, i'd like to get the ModuleBaseAddress of WoW MainModule in C++

i'm using this function:

BaseAddr = GetModuleBaseAddress(PROC_ID, "Wow.exe");
cout << "WoW MainModule located at: " << BaseAddr << endl;

DWORD CMemory::GetModuleBaseAddress(DWORD iProcId, char* DLLName)
{
  HANDLE hSnap;
  MODULEENTRY32 xModule;
  hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, iProcId);
  xModule.dwSize = sizeof(xModule);
  if (Module32First(hSnap, &xModule))
  {
    while (Module32Next(hSnap, &xModule))
    {
        if (strcmp((char*)xModule.szModule, DLLName) == 0)
        {
			CloseHandle(hSnap);
			return (DWORD)xModule.modBaseAddr;
        }
    }
  }
  CloseHandle(hSnap);
  return 0;
}

I'm opening the process with debug rights:

PROC_HANDLE = OpenProcess(PROCESS_ALL_ACCESS, false, PROC_ID);

But the function above always returns 0... Probably i've got some info lack... Please help!

[_Mike]

Why are you casting MODULEENTRY32::szModule to char*? This makes me think you are using the unicode version of the library. Obviously strcmp will fail then.
Might I suggest using _tcsicmp and wrapping your string literals with the TEXT macro instead.
Edit:
You are also never checking the first module.

[Cypher]

Why are you casting MODULEENTRY32::szModule to char*? This makes me think you are using the unicode version of the library. Obviously strcmp will fail then.
Might I suggest using _tcsicmp and wrapping your string literals with the TEXT macro instead.
Edit:
You are also never checking the first module.

You're better off just using Unicode everywhere imo if your app is Windows-only. Microsoft have started releasing all their new APIs in wide variants only because the ANSI APIs are all more or less deprecated.

EDIT:

PROCESS_ALL_ACCESS requires the SeDebugPrivilege I think... Can't remember. Much easier to just specify the actual flags that you need (and also more 'correct').

Also, you're not checking the handle returned by CreateToolhelp32Snapshot for validity.

[_Mike]

PROCESS_ALL_ACCESS requires the SeDebugPrivilege I think... Can't remember. Much easier to just specify the actual flags that you need (and also more 'correct').

As far as I know SeDebug is only needed when the target process is owned by another account which has a higher "security level" (I don't know the proper word for it) than yours, or if it has set a more restrictive ACL than windows supplies by default. Which wow used to do btw, but doesn't anymore. Maybe that's what you were referring to?
I always assume potential malware when I see a wow hack or bot requiring to be run as administrator. There's no need for them to have access to my entire system just for wow.
Maybe for a general purpose application, like an in-game overlay or something, SeDebug might be needed. But even then, in most cases not.
Agreed on specific flags though. I seem to remember someone, probably you, saying that the value and/or size of PROCESS_ALL_ACCESS had changed between windows versions.

Also, you're not checking the handle returned by CreateToolhelp32Snapshot for validity.

Currently that's not an issue because Module32First checks if the handle is valid before attempting to use it. But I doubt the OP knew that, so we'll blame it on sloppy coding
Also this might change in future API versions, so yes; Always check return values.

[Amrok]

Fixed it.

Can be closed.

[Cypher]

As far as I know SeDebug is only needed when the target process is owned by another account which has a higher "security level" (I don't know the proper word for it) than yours, or if it has set a more restrictive ACL than windows supplies by default. Which wow used to do btw, but doesn't anymore. Maybe that's what you were referring to?
I always assume potential malware when I see a wow hack or bot requiring to be run as administrator. There's no need for them to have access to my entire system just for wow.
Maybe for a general purpose application, like an in-game overlay or something, SeDebug might be needed. But even then, in most cases not.
Agreed on specific flags though. I seem to remember someone, probably you, saying that the value and/or size of PROCESS_ALL_ACCESS had changed between windows versions.

Currently that's not an issue because Module32First checks if the handle is valid before attempting to use it. But I doubt the OP knew that, so we'll blame it on sloppy coding
Also this might change in future API versions, so yes; Always check return values.

Right right, I keep forgetting WoW doesn't modify its ACLs anymore. That was indeed what I was referring to.

Also yeah, it was probably me who said that the flag had changed sizes. It caused a lot of problems for the copy-pasters who updated their SDK but weren't setting the WINVER macros accordingly. So their program was compiled only for Vista+ (with the new flag).

P.S. I knew it didn't really matter because Module32First checks regardless, but I still consider it sloppy.