Click here to Skip to main content
11,415,046 members (76,489 online)
Rate this: bad
good
Please Sign up or sign in to vote.
See more: C++ memory
Ive got some code that compares a file on the hard drive to the loaded memory copy and compares them. If it finds any differences i'm trying to overwrite the "Bad" memory with the original "Good" memory but it's crashing on me.

It works because i can detect changes and it will log that there are changes in the correct place when i manually alter them. However i can't seem to overwrite them!

It Crashes for me on this section:

//Replaces 'BAD' code with 'GOOD' code

Here's what i got so far:


if( ReadBuffer )
{
	if( ReadFile( hFile, ReadBuffer, ReadSize, &ReadBytes, NULL ) )
	{
		//These are calculated prior and are fine
		DWORD Size	= CodeSize;
		DWORD Data	= CodeBase;
		DWORD Offset	= CodeOffset;
 
		//Library file Alignments are always 200h, anything else is an executable
		//Have to do this as xp file RVA is ok but Vista+ RVA fails to calculate...
		if( OptionalHeader->FileAlignment == 0x200 )
		{
			//Dll File Data
			Data	=	DataBase;
			Size	=	CodeSize - DataOffset;
			Offset	=	DataBase - CodeBase + 0x400;
 
			Log( "\tRVAOffset:   0x%X (%d)", Offset, Offset );
		}
 
		int d = 0;
 
		for( int i = 0; i < Size; ++i )
		{
			unsigned char ModuleData = *( unsigned char* )( Data + i );
			if( ReadBuffer[ i + Offset ] != NULL && ModuleData != NULL && ReadBuffer[ i + Offset ] != ModuleData )
			{
				Log( "\t\tMismatch @ 0x%X [%02X != %02X]", Data + i, ReadBuffer[ i + Offset ], ModuleData );
				++d;
 
				if( d > 3 )
				{
					//Log( "\tConsecutive mismatches!" );
					ReturnValue = false;
 
					//Replaces 'BAD' code with 'GOOD' code
					for( int j = 1; j < d; ++j )
					{
						unsigned char p = *( unsigned char* )( Data + i - j );
						Log( "\t\t0x%08X [0x%02X] -> [0x%02X]", Data + i - j, p, ReadBuffer[ i + Offset - j ] );
 
						DWORD PageFlag;
						VirtualProtect( (LPVOID)( Data + i - j ), j, PAGE_EXECUTE_READWRITE, &PageFlag );
						*( unsigned char* )( Data + i - j ) = ReadBuffer[ i + Offset - j ];
						VirtualProtect( (LPVOID)( Data + i - j ), j, PageFlag, &PageFlag );
					}
				}
 
				continue;
			}
 
			d = 0;
		}
	}
	else
	{
		Log( "ReadFile Failed %d", GetLastError( ) );
	}
 
	free( ReadBuffer );
}
Posted 17-May-12 5:18am
Comments
Richard MacCutchan at 17-May-12 11:02am
   
Looks like you are messing with virtual memory rather than writing on a disk so it may be an issue of permissions or access level.
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 1

If all you're doing is reading a file, comparing it to some other version of it and then overwriting the differences why not just blat the correct one over the incorrect one after you've done the comparison?

Your code seems well complicated for what you're trying to do. If you simplify your code the error may leap out at you.

- playing around directly with the virtual memory API looks like overkill unless you're going to map the file into memory and then do the comparison. Which you're not doing. If you do attempt something like that consider using std::compare rather than rolling your own comparison

- C style casts are really confusing the issue, try getting rid of as many casts as possible and burying the rest in functions the code would be easier to read. And use C++ casts, it's easier to see dodgy stuff - like the cast of Data to a pointer. In C++ if you convert an integral type to a pointer the only thing you're guaranteed to be able to do with it safely is cast it back to the same type it was originally

- Same goes for some of those conditionals, bung them in a function

- Rename your variables to more meaningful names - e.g. d looks like it should be called consecutive_mismatches

- Be suspicious of any loop that uses continue, especially when it looks like an else of else if would give you the same effect (i.e. setting d to zero)

- You're mixing pointer arithmetic with array indexing. Use array addressing wherever you can or preferably use std::compare. std::compare which has the additional advantage that you don't need to explicitly read the file into memory.

Cheers,

Ash

PS: If you want to prevent tampering write a device driver. You can look anywhere in memory then and you don't have to prod about trying to read another process's memory. You can then do something like hash each code page and check that it's the same periodically. If not, reload and relocate it from the original EXE.

Or just buy a development license for an anti-virus engine which'll let you do most of that stuff without you having to write so much code.
  Permalink  
v4
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 2

Sorry not quite. It's to avoid manipulation. For example.

I run notepad.exe. I then run my app. My app then compares notepad.exe on the hard drive to notepad.exe in memory. If the notepad.exe in memory differs to the hard drive version it replaces the altered bytes with the correct bytes.

Does that make more sense?
  Permalink  
v2
Comments
Richard MacCutchan at 17-May-12 13:14pm
   
That sounds like a very bad thing to do. The image on disk will almost never be the same as the image in memory, as things often get changed as soon as the program gets loaded.
Aescleal at 17-May-12 17:49pm
   
If you ignore imports and relocations what you find on the disk is pretty similar to the _TEXT section of the program. The data is going to be completely different and that's where a bit of malware will be doing it's dirty deeds from.
Richard MacCutchan at 18-May-12 4:08am
   
Sounds like a largely pointless exercise to me. If your PC gets hit by a virus or malware then chances are that the disk image will be corrupt.
Aescleal at 18-May-12 10:19am
   
I'd say so as well, but hey, if someone wants a job in the security industry then doing this sort of project - even if it fails miserably - would help show he knows some of the issues.
Richard MacCutchan at 18-May-12 12:15pm
   
Sounds reasonable.
sjsteve33171 at 18-May-12 10:54am
   
What I'm trying to accomplish is anti code injection. So the code section is all I need to worry about. It's just over writing so injection appears like it fails or didn't work although it did.

So if someone injects another library into the process that for arguments sakes alters a 'je' to a 'jnz' my program will see that and put it back to a 'je' by reading from teh versio on disk. Thus the process thats open in memory the end user sees remains as it should.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
0 Sergey Alexandrovich Kryukov 9,200
1 OriginalGriff 7,512
2 Maciej Los 3,710
3 Abhinav S 3,298
4 Peter Leow 3,084


Advertise | Privacy | Mobile
Web01 | 2.8.150427.4 | Last Updated 17 May 2012
Copyright © CodeProject, 1999-2015
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100