[C++] GetModuleBase incompability

[Amrok]

Hi, i've made a little tool to read / write to wow memory but on some computers GetModuleBase fails! It gets wrong Module Base while others work fine. C++ Redistributable is static linked. This is my function:

UINT_PTR GetModuleBase(LPTSTR lpModuleName, DWORD dwProcessId)
{
	MODULEENTRY32 lpModuleEntry = {0};
	HANDLE hSnapShot = CreateToolhelp32Snapshot(TH32CS_SNAPMODULE, dwProcessId);
	if(!hSnapShot)
		return NULL;
	lpModuleEntry.dwSize = sizeof(lpModuleEntry);
	BOOL bModule = Module32First(hSnapShot, &lpModuleEntry);
	while(bModule)
	{
		if(!_tcscmp(lpModuleEntry.szModule, lpModuleName))
		{
			CloseHandle(hSnapShot);
			return (UINT_PTR)lpModuleEntry.modBaseAddr;
		} 
		bModule = Module32Next(hSnapShot, &lpModuleEntry);
	}
	CloseHandle(hSnapShot);
	return NULL;
}

Can somebody explain why it doesn't work for some users.. it displays 0x400000 als mainmodule on some computers which is WRONG.

[LogicWin]

You could copy'n paste chyper's "Hades-lib"

[_Mike]

Try using the hModule member instead. That has always worked for me. In fact I had never even noticed there was a .modBaseAddr before you mentioned it just now

[Amrok]

Thx .. worked Didnt thought that .modBaseAddr has different effects on some computers. Dont know what was causing it.

Now i only have 1 more problem to solve.. I need to read a string from memory. I've made a loop to read every single byte and check if its 0x00 terminator and keep reading until we found 0x00. Here is the code:

string ReadString(UINT_PTR Offset)
{
	int count;
	char Value[50] = "\0";
	BYTE strbyte = {0x20}; // hack.. anything else than 0x00 to initiate.
	for(count=0;strbyte!=0x00;count++) // keep reading until we found end of string byte 0x00
		if(ReadProcessMemory(hProcess, (LPCVOID)(Offset+count), &strbyte, 1, NULL)) // read single byte and write it to array
			Value[count] = strbyte;
	return (string)Value;
}

I'm sure this can be optimized .. But could you take a look at it and tell me what you think?

[_Mike]

My suggestion; Read more data at once than a single byte, say a full memory page or so, and process the string locally. If you haven't found a '\0' by then, read the next page as well..
The increased time it takes to copy the extra amount of data is probably less than the overhead of repeatedly calling ReadProcessMemory unless the strings are only a few characters long.

Edit:
Also, why are you limiting your strings to 49 characters? Use a vector<char> or similar.
And even worse, you aren't enforcing the 49 char limit so you are risking buffer overflows on long strings.

[Cypher]

My suggestion; Read more data at once than a single byte, say a full memory page or so, and process the string locally. If you haven't found a '\0' by then, read the next page as well..
The increased time it takes to copy the extra amount of data is probably less than the overhead of repeatedly calling ReadProcessMemory unless the strings are only a few characters long.

Edit:
Also, why are you limiting your strings to 49 characters? Use a vector<char> or similar.
And even worse, you aren't enforcing the 49 char limit so you are risking buffer overflows on long strings.

Obviously he's running ring-0 proxies to protect against buffer overflows.

[flo8464]

Also, why are you limiting your strings to 49 characters? Use a vector<char> or similar.

He's using C, at least I hope that.
Please tell me that the return value type 'string' is in no way related to std::string.

[_Mike]

He's using C, at least I hope that.

Read the topic title ;)

[suicidity]

I'm saddened.

[flo8464]

I was a bit bored, so I cleaned up your mess a bit.
It's still inefficient (reading only a byte at a time), but at least it doesn't violate half of C++'s language rules within 10 lines of code.

std::string ReadString(std::uintptr_t address)
{
  std::string temp;

  char current;
  do
  {
    SIZE_T bytesRead;
    BOOL ec = ReadProcessMemory(  hProcess,
                                  reinterpret_cast<LPCVOID>(address++),
                                  static_cast<LPVOID>(&current),
                                  sizeof(current),
                                  &bytesRead);
    if(!ec || bytesRead != sizeof(current))
      throw std::runtime_error("Error reading string");

    if(current)
      temp.push_back(current);
  }
  while(current);

  return temp;
}

[Amrok]

I was a bit bored, so I cleaned up your mess a bit.
It's still inefficient (reading only a byte at a time), but at least it doesn't violate half of C++'s language rules within 10 lines of code.

std::string ReadString(std::uintptr_t address)
{
  std::string temp;

  char current;
  do
  {
    SIZE_T bytesRead;
    BOOL ec = ReadProcessMemory(  hProcess,
                                  reinterpret_cast<LPCVOID>(address++),
                                  static_cast<LPVOID>(¤t),
                                  sizeof(current),
                                  &bytesRead);
    if(!ec || bytesRead != sizeof(current))
      throw std::runtime_error("Error reading string");

    if(current)
      temp.push_back(current);
  }
  while(current);

  return temp;
}

Thanks for your correction. I've written a function to read my whole memory page from beginning of string and then process it in my application. Didn't care too much about inefficiency b4.

To Topic: not hModule Member was causing it! it was _tcscmp! it doesnt work correct on some computers for some reason. I'm now using wcscmp instead!

Also modBaseAddress member is correct:

modBaseAddr
The base address of the module in the context of the owning process.

[Cypher]

I was a bit bored, so I cleaned up your mess a bit.
It's still inefficient (reading only a byte at a time), but at least it doesn't violate half of C++'s language rules within 10 lines of code.

std::string ReadString(std::uintptr_t address)
{
  std::string temp;

  char current;
  do
  {
    SIZE_T bytesRead;
    BOOL ec = ReadProcessMemory(  hProcess,
                                  reinterpret_cast<LPCVOID>(address++),
                                  static_cast<LPVOID>(&current),
                                  sizeof(current),
                                  &bytesRead);
    if(!ec || bytesRead != sizeof(current))
      throw std::runtime_error("Error reading string");

    if(current)
      temp.push_back(current);
  }
  while(current);

  return temp;
}

You should template it on character type (and probably traits and allocator too, but meh, nobody seems to do that anyway ).

[Amrok]

You should template it on character type (and probably traits and allocator too, but meh, nobody seems to do that anyway ).

I have template functions for Read / Write Offset if you mean that. I handle strings as an extra function.

EDIT:
I'm stupid. Didnt read correct. Sorry.

[Cypher]

I have template functions for Read / Write Offset if you mean that. I handle strings as an extra function.

EDIT:
I'm stupid. Didnt read correct. Sorry.

template <typename CharT>
std::basic_string<CharT> ReadString(PVOID Address);

[lanman92]

How would you use a template like that in c++? I know in C# you use Type.Typecode, what is the equal in c++?