Check if memory is readable or why do it not catches the exception?

Question

I have this code that gets called from an injected DLL from a foreign process. It sould read some memory ranges but I sometimes get a segmentation fault at this line DataBuffer := TCharPointer(Address + CharOffset)^;. So is there any way to check if the memory is readable?

function GetCurrentData(Address: Pointer): PChar;
var
  DataBuffer: Char;
  CharArray: Array of Char;
  CharOffset: Integer;
  ReadBytes: longword;
begin
  CharOffset := 0;
  SetLength(CharArray, 0);
  repeat
    DataBuffer := TCharPointer(Address + CharOffset)^;
    CharOffset := CharOffset + 1;
    SetLength(CharArray, CharOffset);
    CharArray[CharOffset - 1] := DataBuffer;
  until (Ord(DataBuffer) = 0);
  Result := PChar(@CharArray[0]);
end;

i also tryed to catch the exception but for some reason this is not working. The host programm still crashes.

unit UnitEventBridgeExports;

{$mode objfpc}{$H+}

interface

uses
  Classes, SysUtils, Windows, ShellAPI, JwaTlHelp32, SimpleIPC;

type
  TCharPointer = ^Char;

const
  WOWEXE = 'TestProgramm.exe';

var
  IPCClient: TSimpleIPCClient;
  PID: DWord;
  Process: THandle;

procedure EventCalled;
procedure InitializeWoWEventBridge; stdcall;

implementation


function GetProcessIDByName(Exename: String): DWord;
var
  hProcSnap: THandle;
  pe32: TProcessEntry32;
begin
  Result := 0;
  hProcSnap := CreateToolHelp32SnapShot(TH32CS_SNAPPROCESS, 0);
  if hProcSnap <> INVALID_HANDLE_VALUE then
  begin
    pe32.dwSize := SizeOf(ProcessEntry32);
    if Process32First(hProcSnap, pe32) = True then
    begin
      while Process32Next(hProcSnap, pe32) = True do
      begin
        if pos(Exename, pe32.szExeFile) <> 0 then
          Result := pe32.th32ProcessID;
      end;
    end;
    CloseHandle(hProcSnap);
  end;
end;


procedure InitializeEventBridge; stdcall;
begin
  IPCClient := TSimpleIPCClient.Create(nil);
  IPCClient.ServerID := 'EventBridgeServer';
  IPCClient.Active := True;
  IPCClient.SendStringMessage('init');
  PID := GetProcessIDByName(EXE);
  Process := OpenProcess(PROCESS_ALL_ACCESS, False, PID);
end;


function GetCurrentData(Address: Pointer): PChar;
var
  DataBuffer: Char;
  CharArray: Array of Char;
  CharOffset: Integer;
  ReadBytes: longword;
  CharPointer: TCharPointer;
  BreakLoop: Boolean;
begin
  CharOffset := 0;
  SetLength(CharArray, 0);
  BreakLoop := False;
  repeat
    try
      CharPointer := TCharPointer(Address + CharOffset);
      DataBuffer := CharPointer^;
      CharOffset := CharOffset + 1;
      SetLength(CharArray, CharOffset);
      CharArray[CharOffset - 1] := DataBuffer;
    except
      BreakLoop := True;
    end;
  until (Ord(DataBuffer) = 0) or BreakLoop;
  Result := PChar(@CharArray[0]);
end;


procedure EventCalled;
var
  TmpAddress: Pointer;
  StringData: PChar;
begin
  {$ASMMODE intel}
  asm
    mov [TmpAddress], edi
  end;
  StringData := GetCurrentData(TmpAddress);
  IPCClient.SendStringMessage('update:' + StringData);
  //IPCClient.SendStringMessage('update');
end;

end.

You're doing it wrong. Find out which ranges of memory are the ones you want, and then read only those ranges. Don't go poking around in memory you're not familiar with. — Rob Kennedy, Jul 24 '12 at 17:34
the try..finally block is not doing anything in the finally part — , Jul 24 '12 at 17:38
You're not trying to catch the exception at all. `Try..finally` is not an exception handler. Use `try..except` for exceptions, and actually **handle** the exception; if you're doing nothing, don't catch it at all. — Ken White, Jul 24 '12 at 17:43
ok is it now better?. because i still get segmentation fault's — Maximilian Ruta, Jul 24 '12 at 17:49
@RobKennedy i want to read a string. so i dont know the end of it. so i nead to start at the start address and loop till i get a null char. — Maximilian Ruta, Jul 24 '12 at 17:51
But you know the *beginning* of the string is safe to read, right? So start reading there, and read a byte at a time (or a page at a time, since that's the allocation and protection granularity) until you find the end. Don't read the next page until you know the last byte of the previous page wasn't the end of the string. — Rob Kennedy, Jul 24 '12 at 18:46
this is what i am doing in the code above. But it still triggers exceptions some times. — Maximilian Ruta, Jul 24 '12 at 18:53
If your last comment was to me, then that's *not* what you're doing in the code. You receive an address (from somewhere; you don't show how) and you read until you find a zero character. You give no regard for how much of the memory is valid — you don't know where the end of the page is. A string can straddle a page boundary. When you reach the end of the page, you can't just keep reading because you probably haven't copied that page from the other process. — Rob Kennedy, Jul 24 '12 at 19:33
@RobKennedy so ok in the code example in my question i show that i get the address from the edi register. but how to know that i am in the memory page? — Maximilian Ruta, Jul 24 '12 at 19:46
You could call `VirtualQuery` to discover the actual bounds of the page (as well as the protection state, for that matter). Where is this address coming from? What do you think is in the EDI register? Who calls `EventCalled`, and how does it know to put something special in EDI? — Rob Kennedy, Jul 24 '12 at 19:51
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/14363/discussion-between-maximilian-ruta-and-rob-kennedy) — Maximilian Ruta, Jul 24 '12 at 19:56

score 2 · Accepted Answer · answered Jul 24 '12 at 20:03

Your GetCurrentData() implementation is returning a pointer to a local array that goees out of scope when the function exits, then EventCalled() tries to use that poiner after it is no longer valid. Try this instead:

function GetCurrentData(Address: Pointer): AnsiString; 
var 
  Offset: Integer; 
begin 
  Result := '';
  Offset := 0; 
  repeat 
    try 
      if PByte(Longint(Address) + Offset)^ = #0 then Break;
      Inc(Offset); 
    except 
      Break; 
    end; 
  until False; 
  SetString(Result, PAnsiChar(Address), Offset); 
end; 

procedure EventCalled; 
var 
  TmpAddress: Pointer; 
  StringData: AnsiString; 
begin 
  {$ASMMODE intel} 
  asm 
    mov [TmpAddress], edi 
  end; 
  StringData := GetCurrentData(TmpAddress); 
  IPCClient.SendStringMessage('update:' + StringData); 
  //IPCClient.SendStringMessage('update'); 
end;

Oh. I thought that PChar is an datatype and not a pointer. I could test you code when i am back home tomorrow — Maximilian Ruta, Jul 25 '12 at 10:31

score 1 · Answer 2 · edited May 23 '17 at 11:48

1

IsBadReadPtr API is here to help. You give address and size, and you get the readability back. Raymond Chen suggests to never use it though.

Other than that, VirtualQuery should give you information about the address in question to tell its readability.

Since Ken in comments below re-warned about danger of IsBadReadPtr, I bring it up to the answer to not pass by. Be sure to read the comments and links to Raymdond's blog. Be sure to see also:

edited May 23 '17 at 11:48

Community

1
1

answered Jul 24 '12 at 17:03

Roman R.

68,205
6
94
158

1

`IsBadReadPtr` is bad, and you shouldn't use it. See Raymond Chen's blog posts [here](http://blogs.msdn.com/b/oldnewthing/archive/2006/09/27/773741.aspx) and [here](http://blogs.msdn.com/b/oldnewthing/archive/2007/06/25/3507294.aspx) for reasons why this is the case. – Ken White Jul 24 '12 at 17:06
1

True, and I mentioned it, however we don't know the purpose, and for debugging reasons `IsBadReadPtr` is just fine. – Roman R. Jul 24 '12 at 17:08
No, it isn't. Read the two posts I linked to for reasons why, as I said. – Ken White Jul 24 '12 at 17:10
Raymond's warning is about "You just blew your one chance." and this instructs to stay away from this API. On debugging scenarios you might want to not care about second chance, and the API is readily available for you to hit an exception and bring up the problem. Now why not use it? It is all here at one's fingertips. – Roman R. Jul 24 '12 at 17:16
I gave you two separate links, both of which say "Don't use this function" and written by a long time MS employee on the Shell team (who is extremely knowledgeable, BTW), the API documentation itself says "You shouldn't use this function", and you still insist on advising to use the function? Sorry; I have to disagree. The fact that there's a saw available doesn't automatically mean you should use it to cut off your arms and legs. The "one chance" is important, and the post explains exactly why; it doesn't matter if it's for debugging or not, the fact you used the function breaks it's validity – Ken White Jul 24 '12 at 17:41
Raymond's second post (which you didn't add to your edit when you added the first one I linked to) explains why there is no better alternative as well. When someone provides me with multiple links to show I'm providing the wrong answer, I always read all of them to make sure I understand what they're saying fully, and then decide whether I should continue to disagree. :-) – Ken White Jul 24 '12 at 17:46
Raymon'd point is that once `IsBadXxx` reports something bad, along with this the whole process turns sour. I accept this, but does this make the API unusable? No. I cannot remember any pattern on my code where it is used not as a part of `assert`, but there it is perfectly on purpose there. Once something is bad, would you "just crash" or rather be able to save stuff or report issue? Definitely the latter. Did MS use and suggest these functions? Yes, I remember massive use on DirectX SDK at the very least. – Roman R. Jul 24 '12 at 19:27

Check if memory is readable or why do it not catches the exception?

2 Answers2