I'm currently getting garbage data when using ReadProcessMemory to read from another process.

This is my definition:
BOOL ReadProcessMemory(HANDLE hProcess, LPCVOID lpBaseAddress, LPVOID lpBuffer, SIZE_T nSize, SIZE_T *lpNumberOfBytesRead);

And I'm reading it like this:
if (!ReadProcessMemory(process,
      cast(PCVOID)address, cast(PVOID)&data,
      cast(DWORD)stringSize, cast(PDWORD)&bytesRead)) {
      return defaultValue;
    }

process is a HANDLE that I got from OpenProcess()
address is a DWORD
data is char[1024]
stringSize is size_t
bytesRead is PDWORD

The address I obtained was from Ollydbg and it's a static address. I'm not sure if I however need some kind of offset etc.

I tried to search and found you might have to use GetModuleHandleA() but I tried that and I still get garbage data with the offset from that.

What am I doing wrong?

On Friday, 30 June 2017 at 20:14:15 UTC, bauss wrote:
> This is my definition:
> BOOL ReadProcessMemory(HANDLE hProcess, LPCVOID lpBaseAddress, LPVOID lpBuffer, SIZE_T nSize, SIZE_T *lpNumberOfBytesRead);
>
> And I'm reading it like this:
> if (!ReadProcessMemory(process,
>       cast(PCVOID)address, cast(PVOID)&data,
>       cast(DWORD)stringSize, cast(PDWORD)&bytesRead)) {
>       return defaultValue;
>     }

I guess the first cast is necessary when `address` isn't typed as a pointer yet. But the other casts shouldn't be needed. If you get errors without them, those errors might give a hint on what's wrong.

> process is a HANDLE that I got from OpenProcess()
> address is a DWORD
> data is char[1024]
> stringSize is size_t
> bytesRead is PDWORD

bytesRead is a SIZE_T, no? Or maybe a DWORD.

On Friday, 30 June 2017 at 21:36:25 UTC, ag0aep6g wrote:
> On Friday, 30 June 2017 at 20:14:15 UTC, bauss wrote:
>> This is my definition:
>> BOOL ReadProcessMemory(HANDLE hProcess, LPCVOID lpBaseAddress, LPVOID lpBuffer, SIZE_T nSize, SIZE_T *lpNumberOfBytesRead);
>>
>> And I'm reading it like this:
>> if (!ReadProcessMemory(process,
>>       cast(PCVOID)address, cast(PVOID)&data,
>>       cast(DWORD)stringSize, cast(PDWORD)&bytesRead)) {
>>       return defaultValue;
>>     }
>
> I guess the first cast is necessary when `address` isn't typed as a pointer yet. But the other casts shouldn't be needed. If you get errors without them, those errors might give a hint on what's wrong.
>
>> process is a HANDLE that I got from OpenProcess()
>> address is a DWORD
>> data is char[1024]
>> stringSize is size_t
>> bytesRead is PDWORD
>
> bytesRead is a SIZE_T, no? Or maybe a DWORD.

It's the same.

This is my read function:
string ReadWinString(HANDLE process, DWORD address, size_t stringSize, string defaultValue = "") {
  if (!process || !address) {
    return defaultValue;
  }

  SIZE_T bytesRead;
  char[1024] data;

  if (!ReadProcessMemory(process,
    cast(PCVOID)address, cast(PVOID)&data,
    stringSize, &bytesRead)) {
    return defaultValue;
  }

  auto s = cast(string)data[0 .. stringSize];

  return s ? s : defaultValue;
}

And this is how I call it:
auto text = ReadWinString(handleFromOpenProcess, 0x0000000, 16, "defaultString...");

where 0x0000000 is the address obviously.

If you can spot what I'm doing wrong it would be appreciated.

On Friday, 30 June 2017 at 23:41:19 UTC, bauss wrote:
> On Friday, 30 June 2017 at 21:36:25 UTC, ag0aep6g wrote:
>> On Friday, 30 June 2017 at 20:14:15 UTC, bauss wrote:
>>>     [...]
>>
>> I guess the first cast is necessary when `address` isn't typed as a pointer yet. But the other casts shouldn't be needed. If you get errors without them, those errors might give a hint on what's wrong.
>>
>>> [...]
>>
>> bytesRead is a SIZE_T, no? Or maybe a DWORD.
>
> It's the same.
>
> This is my read function:
> string ReadWinString(HANDLE process, DWORD address, size_t stringSize, string defaultValue = "") {
>   if (!process || !address) {
>     return defaultValue;
>   }
>
>   SIZE_T bytesRead;
>   char[1024] data;
>
>   if (!ReadProcessMemory(process,
>     cast(PCVOID)address, cast(PVOID)&data,
>     stringSize, &bytesRead)) {
>     return defaultValue;
>   }
>
>   auto s = cast(string)data[0 .. stringSize];
>
>   return s ? s : defaultValue;
> }
>
> And this is how I call it:
> auto text = ReadWinString(handleFromOpenProcess, 0x0000000, 16, "defaultString...");
>
> where 0x0000000 is the address obviously.
>
> If you can spot what I'm doing wrong it would be appreciated.

I mean I get data, it's not like the call fails or gives an error. It's just not the data I'm expecting.

I suspect the address is wrong, but it's the static address I picked up from ollydbg, so I'm kinda lost as for how ollydbg can get the correct string and I get the wrong one using same address.

On Friday, 30 June 2017 at 23:53:19 UTC, bauss wrote:
>
> I suspect the address is wrong, but it's the static address I picked up from ollydbg, so I'm kinda lost as for how ollydbg can get the correct string and I get the wrong one using same address.

You are aware that processes life in different memory spaces ?

On Friday, 30 June 2017 at 23:56:10 UTC, Stefan Koch wrote:
> On Friday, 30 June 2017 at 23:53:19 UTC, bauss wrote:
>>
>> I suspect the address is wrong, but it's the static address I picked up from ollydbg, so I'm kinda lost as for how ollydbg can get the correct string and I get the wrong one using same address.
>
> You are aware that processes life in different memory spaces ?

Well it's a static address I'm trying to read from, so it shouldn't matter if I have the write handle to the process and the static address, should it?

On 07/01/2017 01:41 AM, bauss wrote:
> string ReadWinString(HANDLE process, DWORD address, size_t stringSize, string defaultValue = "") {
>    if (!process || !address) {
>      return defaultValue;
>    }
> 
>    SIZE_T bytesRead;
>    char[1024] data;
> 
>    if (!ReadProcessMemory(process,
>      cast(PCVOID)address, cast(PVOID)&data,

The second cast still looks suspicious. PVOID is void*, right? Then any mutable pointer type should implicitly convert to PVOID and you shouldn't need the cast.

>      stringSize, &bytesRead)) {
>      return defaultValue;
>    }
> 
>    auto s = cast(string)data[0 .. stringSize];
> 
>    return s ? s : defaultValue;

Here's an error that produces garbage.

`data` is a fixed-sized array, so the values are on the stack. That means `s` points to the stack. You can't return a pointer to the stack. It becomes invalid when the function returns. You can put it on the heap instead: `auto s = data[0 .. stringSize].idup;`.

> }

On Sat, Jul 01, 2017 at 02:23:36AM +0200, ag0aep6g via Digitalmars-d-learn wrote:
> On 07/01/2017 01:41 AM, bauss wrote:
[...]
> >      stringSize, &bytesRead)) {
> >      return defaultValue;
> >    }
> > 
> >    auto s = cast(string)data[0 .. stringSize];
> > 
> >    return s ? s : defaultValue;
> 
> Here's an error that produces garbage.
> 
> `data` is a fixed-sized array, so the values are on the stack. That means `s` points to the stack. You can't return a pointer to the stack. It becomes invalid when the function returns. You can put it on the heap instead: `auto s = data[0 .. stringSize].idup;`.
[...]

The compiler is supposed to catch errors like these with -dip1000. Recently there was a DIP1000-related fix checked in that fixed some of the problems with -dip1000 (specifically, a linker error I was running into), so you may want to consider compiling with -dip1000 if you're running the latest compiler.


T

-- 
Klein bottle for rent ... inquire within. -- Stephen Mulraney

On Saturday, 1 July 2017 at 00:23:36 UTC, ag0aep6g wrote:
> On 07/01/2017 01:41 AM, bauss wrote:
>> string ReadWinString(HANDLE process, DWORD address, size_t stringSize, string defaultValue = "") {
>>    if (!process || !address) {
>>      return defaultValue;
>>    }
>> 
>>    SIZE_T bytesRead;
>>    char[1024] data;
>> 
>>    if (!ReadProcessMemory(process,
>>      cast(PCVOID)address, cast(PVOID)&data,
>
> The second cast still looks suspicious. PVOID is void*, right? Then any mutable pointer type should implicitly convert to PVOID and you shouldn't need the cast.
>
>>      stringSize, &bytesRead)) {
>>      return defaultValue;
>>    }
>> 
>>    auto s = cast(string)data[0 .. stringSize];
>> 
>>    return s ? s : defaultValue;
>
> Here's an error that produces garbage.
>
> `data` is a fixed-sized array, so the values are on the stack. That means `s` points to the stack. You can't return a pointer to the stack. It becomes invalid when the function returns. You can put it on the heap instead: `auto s = data[0 .. stringSize].idup;`.
>
>> }

Using ".idup" makes no difference in the result. I was under the impression the cast would already do that though, guess not. However the result is the same. I also tried to check "data" directly and it's already garbage there.

Well the address is not a pointer. It's DWORD which is uint, so the cast is necessary since it stores the address.

On 07/01/2017 02:30 AM, bauss wrote:
> On Saturday, 1 July 2017 at 00:23:36 UTC, ag0aep6g wrote:
>> On 07/01/2017 01:41 AM, bauss wrote:
[...]
>>>    if (!ReadProcessMemory(process,
>>>      cast(PCVOID)address, cast(PVOID)&data,
>>
>> The second cast still looks suspicious. PVOID is void*, right? Then any mutable pointer type should implicitly convert to PVOID and you shouldn't need the cast.
[...]
> Well the address is not a pointer. It's DWORD which is uint, so the cast is necessary since it stores the address.

Not that one. The other one. This one: `cast(PVOID)&data`.

I don't expect it to be related to your problem, but it shouldn't be necessary as far as I see.