Changing a global variable in a thread

[DjSt3rios]

Hello. I have a small problem with my program. It is kinda a mess but I will try to explain you what I am trying to do. I have some threads. One of it, it attempts to detect a game client. So my code is sort of like that:

DWORD ProcessID; // The process ID of the game client

void test()
{
     char* text;
     sprintf(text, "proccess ID is: %d",ProcessID); // and this does NOT work. Empty Message..
     MessageBox(0, text, "Process ID Test", 0);
}

void DetectClient(void *pParams) // This is a thread..
{
     while(notfound)
     {
                LhWnd = FindWindow(NULL, "Lineage II");
                DWORD TPID = GetWindowThreadProcessId(LhWnd,&ProcessID); // Here the ProcessID being set
                if(TPID == 0)
                {
                    Sleep(2000);
                }
                else
                {
                        char* text;
                        sprintf(text, "proccess ID is: %d",ProcessID); // This works fine.
                        MessageBox(0, text, "Process ID Test", 0);
                        notfound = false;
                }
                
     }
     test(); // Test function being called 
     _endthread();
}

this is the main idea, which is copy/paste most of that. So basicly, its like the variable changes, but only inside the thread... why does that happen? Thanks in advance. (P.S I am pretty new to C++)

[Zhuge]

On the surface, it looks like it should work. Do you have any other threads running that try to change the ProcessID global? If you just add a main() that runs this thread function just it still fail?

[DjSt3rios]

I do have threads, however, none of them change the variable. I searched the whole thing, it only uses it for memory scanning etc. Actually it used to work great but I made a few more threads and seems that is not working right now. Also, I just tried what you says, and it fails.. even before the while stops (actually its a copy paste from the above), it still shows an empty message which means it failed... I have about 5 threads working, maybe they are too many..

[Zhuge]

Actually I just noticed something very suspect. Before your sprintf calls, you create a char* to hold the text you generate from the call...but you never allocate any memory or even initialize that C-string!

So when you call sprintf() you are writing to completely random memory. I'm a bit surprised you aren't segfaulting, but you've probably just been getting lucky. You don't own the memory that you are saving the string to, so it could just be being written over by whatever actually owns the memory.

[JLBorges]

> char* text;
> sprintf(text, "proccess ID is: %d",ProcessID); // This works fine. 

No. It doesn't.

[DjSt3rios]

Well to be honest its the first application I am writing in C++, I am even suprised myself how I managed to do all that (I did a lot of google search though)... so is there any advice you guys can give me? How should I proceed? I am not even sure how to allocate memory... any help would be appriciated.

P.S Nevermind, seems like I found what you meant. I changed:
char* text;
to
char text[20];

and looks like it prints fine now.. I should be more careful.. now I realised what you meant by memory allocation... I will try to fix some things on my application and i will let you know, thanks a lot everyone.

[JLBorges]

char text[ /* 20 */ 128] ; // create an array large enough to hold the string
sprintf(text, "proccess ID is: %d",ProcessID); // sprintf places characters into it 

[DjSt3rios]

Thanks for all the help, my messages show up correctly this time, However, I still have the same problem. All my threads pick up the correct Process ID the first time(Everytime the treads run, they show a message). 2 of my threads which i have the debug messsage, show it correctly. However, When I click OK and it runs again, the Process ID becomes 6946884, this specific number, even if i restart the application, it still shows this specific number...

EDIT: That's really weird. I changed:
DWORD ProcessID;
with
DWORD ProcessID = 3;

and it works perfectly... now the value is being changed how it should, and it doesn't change back to 6946884. Maybe the value was being unassigned somehow, although it still doesn't make sense, what can i say...

[JLBorges]

> Maybe the value was being unassigned somehow

There is an error in some part of your code - a portion of the static data area is being corrupted.

Set a data change breakpoint on ProcessID
Visual Studio: http://msdn.microsoft.com/en-us/library/350dyxd0(v=vs.100).aspx
and run your program under the debugger.

[DjSt3rios]

I am programming with Dev-CPP, i am not using visual studio. I heard NET programs are extremely easy to decompile.. of course that doesn't mean my program can't be decompile but I understand its harder... I will try to do it with Dev-CPP, but i am not sure if it will help.

EDIT: nvm seems like its for native C++. I will try to add my project in Visual Studio.


EDIT2: I did some debugging on my application. I have this code in a thread:

//Global variable
DWORD key;

// Part of thread

            DWORD base = GetModuleBase("File.dll", ProcessID);
            if(base == NULL)
            {
                Sleep(1000);
                continue;
            } 
            DWORD a = 0x620B80;
            DWORD B = base + a;
            unsigned long tempkey;
            if(key != NULL && key != 0 ) 
            {
                tempkey = key;
            }
            HANDLE keyProcess = OpenProcess(PROCESS_ALL_ACCESS, FALSE, ProcessID );
            ReadProcessMemory (keyProcess, (void*)B, &key, 20, 0);
            CloseHandle(keyProcess);

After the debugging, I found out that what's happening is, the code goes till the ReadProcessMemory. After I continue the debug, Instead of stopping in CloseHandle (I had a breakpoint), the thread restarts and the ProcessID becomes 0. I will keep digging to see what's going on, if anyone has any idea please let me know.


The GetModuleBase function(PS. I found it in a public forum)

DWORD GetModuleBase(LPSTR 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(!strcmp( lpModuleEntry.szModule, lpModuleName ) )
        {
            CloseHandle( hSnapShot );
            return (DWORD)lpModuleEntry.modBaseAddr;
        }
        bModule = Module32Next( hSnapShot, &lpModuleEntry );
    }
    CloseHandle( hSnapShot );
    return NULL;
}

Edit: Looks like if I change the amount of bytes to read, it might work properly, or slightly change the ProcessID(like remove 200 from the ID) or turn it into 6946884...

[JLBorges]

> ReadProcessMemory (keyProcess, (void*)B, &key, 20, 0);

key is a DWORD. You can't read 20 bytes into it.

Change to: ReadProcessMemory (keyProcess, (void*)B, &key, sizeof(key), 0);

[DjSt3rios]

Actually I want to read 10 bytes. I changed key to unsigned long, the number might be negative, and I changed the size to 10. I guess it will do, right?

[JLBorges]

> Actually I want to read 10 bytes.

If you want to read 10 bytes, you must have 10 bytes of memory into which it can be read.


> I changed key to unsigned long, the number might be negative,
> and I changed the size to 10. I guess it will do, right?

It won't do at all. You can't read more than sizeof( unsigned long ) bytes into a variable of type unsigned long.

unsigned long key[3] ;

// ...

// Read 3 * sizeof(unsigned long) bytes into key[0], key[1], key[2]
ReadProcessMemory (keyProcess, (void*)B, key, sizeof(key), 0);

[DjSt3rios]

I can't really work it out :/ I need to read a 10 digit number, might be negative, might be positive. can i create a long variable to hold this 10 digit number?

[JLBorges]

> I need to read a 10 digit number, might be negative, might be positive.
> can i create a long variable to hold this 10 digit number?

Depends on the type of variable in which this 10 digit number is stored. You need to create a variable of precisely the same type.

#include <iostream>
#include <windows.h>

int main()
{
    const HANDLE keyProcess = GetCurrentProcess() ;

    const char number_string[] = "-1234567890" ;
    const long number_long = 55555555 ;
    const long long number_long_long = -888888888888 ;
    LARGE_INTEGER number_large_integer ; number_large_integer.QuadPart = 9876543210 ;

    char copy_of_number_string[ sizeof(number_string) ] ;
    ReadProcessMemory( keyProcess, number_string, copy_of_number_string, sizeof(number_string), 0 ) ;
    std::cout << "copy_of_number_string: " << copy_of_number_string << '\n' ;

    long copy_of_number_long ;
    ReadProcessMemory( keyProcess, &number_long, &copy_of_number_long, sizeof(number_long), 0 ) ;
    std::cout << "copy_of_number_long: " << copy_of_number_long << '\n' ;

    long long copy_of_number_long_long ;
    ReadProcessMemory( keyProcess, &number_long_long, &copy_of_number_long_long, sizeof(number_long_long), 0 ) ;
    std::cout << "copy_of_number_long_long: " << copy_of_number_long_long << '\n' ;

    LARGE_INTEGER copy_of_number_large_integer ;
    ReadProcessMemory( keyProcess, &number_large_integer, &copy_of_number_large_integer, sizeof(number_large_integer), 0 ) ;
    std::cout << "copy_of_number_large_integer: " << copy_of_number_large_integer.QuadPart << '\n' ;
}

[DjSt3rios]

hmm I see. Actually I transfered my application Visual Studio, made a few changes to work with it though, but looks like the memory corruption is gone, at least for now. I will let you know as soon as I am sure about it. Thanks a lot.