I am trying to write a subroutine to search for string literals in a remote process. I am pretty sure the wheel is available and I don't think my version is working that well. SO I'd appreciate if you could comment/modify/advice for code improvement, thanks a lot! I have a small tool that's about to finish with this function added. I'll release it asap afterwards.

BOOL searchProcMemory(HANDLE hp, DWORD addr, DWORD dwBytes, LPBYTE sStr,
BOOL literal, HWND listresult){

PBYTE data = 0;
BOOL retval = FALSE;
DWORD dwRead = 0, i, j;

MEMORY_BASIC_INFORMATION mbi;

LPBYTE lp = 0;
BYTE buffer[512];
DWORD found = 0, alen = strlen(sStr);

SendMessage(listresult, LB_RESETCONTENT, 0, 0);

__try{

while(VirtualQueryEx(hp, lp, &mbi, sizeof(MEMORY_BASIC_INFORMATION)) == sizeof(mbi)){
if(!mbi.State&MEM_COMMIT || mbi.Protect&PAGE_NOACCESS
|| (DWORD)mbi.AllocationBase >= 0x70000000){
lp += mbi.RegionSize;
continue;
}
data = malloc(mbi.RegionSize);
if(data == 0){
ListBox_AddString(listresult, "Reading data from remote process failed, cannot allocate";
__leave;
}

retval = ReadProcessMemory(hp, (PDWORD)mbi.BaseAddress, (LPVOID)data,
mbi.RegionSize, &dwRead);
if(retval == FALSE || dwRead != mbi.RegionSize){
sprintf(buffer, "%08X %08X %08X %08X %08X", mbi.AllocationBase, mbi.BaseAddress, mbi.RegionSize, dwRead, data);
ListBox_AddString(listresult, buffer);
ListBox_AddString(listresult, "Reading data from remote process failed, cannot read";
//__leave;
}else{
for(i = 0; i < mbi.RegionSize; i ++){
if(strncmp(data+i, sStr, alen) == 0){
found ++;
if(found < 100){
sprintf(buffer, "0x%08X - %s", (LPBYTE)mbi.AllocationBase+i, sStr);
ListBox_AddString(listresult, buffer);
}else
ListBox_AddString(listresult, "Too many found matches, skipping..";
}
}
}
free(data);
lp += mbi.RegionSize;
}
retval = TRUE;
}
__finally{
if(data) free(data);
}
return retval;
}

I have some idea, hope you will not mind.
1. The listbox will splash many times when found strings added. You can prevent it by SendMessage(listresult, WM_SETREDRAW, FALSE, 0) below the SendMessage(listresult, LB_RESETCONTENT, 0, 0) line to turn off the listbox redraw and turn it on with SendMessage(listresult, WM_SETREDRAW, TRUE, 0) in the finally block.
2. The "if(data) free(data)" can cause your app crash because you double free it in the "free(data); lp += mbi.RegionSize" block.
3. GPF can occur in the block:
for(i = 0; i < mbi.RegionSize; i ++){
if(strncmp(data+i, sStr, alen) == 0){
strncmp function can read exceed mbi.RegionSize memory block.
For safely, I think we need change it to:
for (i = 0; i < mbi.RegionSize - alen; i ++){
if(strncmp(data+i, sStr, alen) == 0){
4. strncmp is a slow function, so if you need for speed, you can use the BooyerMoore algorithm in MASM32.
5. You allocate and free memory many times in the while loop. It will make slowndown your app. I think mbi.RegionSize is a constant, so you can allocate it one time before the while loop and free it in the finally block.
6. I am wonder a string can be overlapped between two pages ? If it is, the string will not be found ?
Best regards,
TQN

Thanks for the sound tips. RegionSize is not fixed so I have to dynamically allocate/free the memory. Speed is a problem sometimes, I can accelerate that a bit because string literals are usually very sparse in a executible memory image.

Sorry homersux, I have a mistake about RegionSize. It is a size of a continuous memory blocks have same protection flags.
I have some little ideas, hope you will dont mind.
- You can add an option to search Unicode string. Just convert sStr to an another local Unicode string variable by MultiByteToWideChar, and use the wstrncmp function to compare.
- Use realloc function instead of malloc. If latter RegionSize is larger than previous size, save this size to a local variable, realloc the memory with new size, and afterall, free that memory block only once time in __finally block.
Wait for your release tool.
Best regards,
TQN