Luigi Auriemma

aluigi.org (ARCHIVE-ONLY FORUM!)
It is currently 19 Jul 2012 14:00

All times are UTC [ DST ]





Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 15 posts ] 
Author Message
 Post subject: Question: Fixing buffer overflows with ASM.
PostPosted: 31 Aug 2007 19:01 

Joined: 21 Aug 2007 17:12
Posts: 28
I know nothing about ASM. But in the game I play, Tribes, it crashes from 1001 different things. Most user-exploited crashes are buffer overflows.

So my question is, how can I spot and fix a places were a user could possible cause a buffer overflow.


Top
 Profile  
 
 
 Post subject:
PostPosted: 31 Aug 2007 20:52 

Joined: 13 Aug 2007 21:44
Posts: 4068
Location: http://aluigi.org
when I need to fix a bof I first find the place where the overflowing happens and then I add my own instructions in the free parts of the code, for example in the sequence of nops and ret which is usually available between two functions (jmp patch, patched code, jmp old code)
Sometimes I optimize directly the existent assembly code gaining space and speed, the best example is mohaaboffix where I saved tons of bytes.

The most difficult part is the finding of the buffer-overflow, in this case you should use a debugger with an EIP tracer or just the Trace function (sloooow) or anything which can help you to know what have been the latest functions called before the crash.

In case of memcpy or strcpy overflow you will see the usage of repz movsd or sometimes just the functions in msvcrt.dll or other dll for copying the data from a buffer to another and in this case you can place a limit to the ecx register, for example doing a AND which is a fast and small fix.
Limiting the "counter" is usually the best choice, anyway I'm talking about a generic case so the best solution depends by the code and type of bug you have to patch.

If you have a piece of assembly code on which you have doubts post it here.


Top
 Profile  
 
 Post subject:
PostPosted: 31 Aug 2007 21:32 

Joined: 13 Aug 2007 21:44
Posts: 4068
Location: http://aluigi.org
I was checking a good example to show and probably I have found the right one:

http://aluigi.org/patches/codmapboffix.lpatch

Here we have a classical buffer-overflow where the output string is just the concatenation of a fixed string (the path) and the one received by the client (the map).

BYTES_ORIGINAL
8B CB ; mov ecx, ebx
8B D1 ; mov edx, ecx
C1 E9 02 ; shr ecx, 02
BF 90 67 9A 01 ; mov edi, 019A6790
F3 A5 ; rep movsd
8B CA ; mov ecx, edx
83 E1 03 ; and ecx, 003
F3 A4 ; rep movsb

here we have EBX which contains the length of the string, this value is then copied in ECX and EDX for copying the data respectively 4 bytes at time and then the remaining bytes.

This is an optimization made by all the compilers for memcpy() since copying one byte at time is a bit slower than 4 bytes (32 bits) at time, the byte-per-byte technique is used only for the remaining bytes since if the string was 7 bytes long there are still 3 bytes to copy.

The difference between memcpy() and memmove() is just that the second one does a one-byte copying for avoiding the overwriting of parts of the string during the moving.

Returning to memcpy here we have a good example of a piece of code which can be optimized for gaining space for a patch since, as you can see, the value of ebx is copied in two registers which are then divided by 4: ECX is the result and EDX is the remainder.
Then the rep movsd instruction will copy the bytes from the buffer pointed by ESI to the one pointed by EDI (019A6790).

The more simple way for gaining bytes is just transforming the memcpy() in a memove(), in short we must remove the 4 bytes copying.

After having made this operation we will have many bytes free to use where is possible to add a more complex check or just the simple AND solution that I like a lot, in fact it takes only 3 bytes (for 8 bits numbers).

Remember that AND is not a real limit, if you set AND 7 (the number is a bitmask) with a string of 5 bytes you will still have 5 but if it's 8 you will have 0 not 7 (the limit), in any case we will have a length lower than that value which is what we want.

The following is my patch, you can notice all the NOP bytes:

BYTES_PATCH
83 E3 3F ; and ebx, 03F
8B CB ; mov ecx, ebx
8B D1 ; mov edx, ecx
90 90 90
BF 90 67 9A 01 ; mov edi, 019A6790
90 90
8B CA ; mov ecx, edx
F3 A4 ; rep movsb


Top
 Profile  
 
 Post subject:
PostPosted: 04 Sep 2007 13:38 

Joined: 13 Aug 2007 21:44
Posts: 4068
Location: http://aluigi.org
another good example of binary patch is the one about haloboom.

In this case there was an off-by-one caused by a set of instruction similar to the following:

function() {
char hash[32];
...
if(enckey[0]) {
for(i = 0; i < 16; i++) {
sprintf(hash + (i * 2), "%02", enckey[i]);
}
hash[i * 2] = NULL;
}

so the off-by-one is just that NULL byte added at the end of the hash buffer which is 32 bytes long, not 33.

The game uses the exception handler (I think this is the name), in short the compiler adds 4 bytes at the end of the stack where it stores an unique number which is checked before returning from the function to know if a buffer-overflow is happened.

So if our canary (the name of this 32 bit field) was 0x11223344 it will become 0x11223300 after the off-by-one and that's the cause of the error message which interrupts the server.

In this case the fix is simple, all we need to do is just using these 4 bytes of the canary as an extension of our hash buffer and for doing this is enough to avoid the final check at the end of the function placing 5 NOPs where is used the CALL.

Why this problem doesn't happen everytime a client joins a server?
The answer is inside the method used by Halo for generating the encryption keys, in fact this method generates non-zero bytes only at the end of the key and so the above instructions will never happen... except when haloboom forces this case.
Example of Halo encryption key, note the first byte which is ever zero:
00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 ac


Top
 Profile  
 
 Post subject:
PostPosted: 03 Oct 2007 06:46 

Joined: 21 Aug 2007 17:12
Posts: 28
MOV DWORD PTR DS:[EDX+7D4],EAX

That is where it crashed for me.

It looks like its a return value for a function.


Top
 Profile  
 
 Post subject:
PostPosted: 03 Oct 2007 07:01 

Joined: 21 Aug 2007 17:12
Posts: 28
http://kigen.ath.cx/crash10032007.bmp


Top
 Profile  
 
 Post subject:
PostPosted: 04 Oct 2007 12:22 

Joined: 13 Aug 2007 21:44
Posts: 4068
Location: http://aluigi.org
you see the bug there but the real bug could happen elsewhere.
In your case seems that EDX points outside the available memory, since it's not a NULL or arbitrary value.
Then depends by the exploit which should help you a bit to understand what type of bug it can be (a buffer-overflow? a failed malloc? writing or reading outside the available memory? format string? and so on)


Top
 Profile  
 
 Post subject:
PostPosted: 05 Oct 2007 00:36 

Joined: 10 Sep 2007 05:40
Posts: 6
struct Player {
...
Player *mSuppressionPrev, *mSuppressionNext; // 0x7d4, 0x7d8
...
};

When the SensorManager is created, it makes a dummy player which is the root of a doubly linked list. When Player::setSensorSupression is called, Player::UnlinkFromSuppressionList(004398C8) is called to unlink the player from the list (if necessary), and then SensorManager::addPlayerToSuppressionList(004398C8) is called to link the player to the list if Player::setSensorSupression was called with a positive value.

Player::UnlinkFromSuppressionList: http://www.team5150.com/~andrew/sensorcrash2.png

Unfortunately, Player::UnlinkFromSuppressionList is also called by Player::Kill and internally by Player::onRemove when the actual Player object is garbage collected, aka 2 more times after the player object has already been removed from the list. You should notice that mSuppressionPrev and mSuppressionNext are NOT null'd out when the player object is unlinked.


The crash happens like so:

- dummy{prev=null, next=null}
[ A activates his sensor jammer pack ]
- A{prev=null,next=dummy} -> dummy{prev=A, next=null}
[ B activates his sensor jammer pack ]
- A{prev=null,next=B} -> B{prev=A,next=dummy} -> dummy{prev=B, next=null}
[ A deactivates his pack ]
- B{prev=null,next=dummy} -> dummy{prev=B, next=null}, !!!A{prev=null,next=B}!!!
[ B dies, his object is garbage collected, and the memory freed before A dies or reactivates his pack. Player::UnlinkFromSuppressionList is called 3 times for B, but nothing happens because B only points at dummy, which still exists ]
[ A dies, Player::UnlinkFromSuppressionList tries to write to A->next->prev aka B->prev and crashes ]

Borland C++ was great at producing redundant code so it's possible to recode the function to null out 7d4/7d8: http://www.team5150.com/~andrew/sensorcrash2_patch.png

Patch: http://www.team5150.com/~andrew/tribes. ... ion.lpatch


You might want to hit CTRL-A in Olly (analyze code) so you can see the program structure. It's also helpful to explain how to reproduce a crash since an EIP and access violation usually don't give enough context unless the crash is trivial to debug.

It only took me half an hour or so to find out what was wrong, but I have much of Tribes labeled in Olly and know how most of it works. The major "breakthrough" was realizing the spot at where it crashed looked like updating pointers in a doubly linked list. After that it was easy to realize it was unhooking the player from the list, that the function was called twice after it was already unlinked (I already had Player::kill and Player::onRemove labeled), and that it might dereference a pointer which no longer exists. I could have set hardware breakpoints on the offsets at 7d4/7d8 in the player object to see what other functions were using them, but it wasn't necessary.


Top
 Profile  
 
 Post subject:
PostPosted: 09 Oct 2007 23:59 

Joined: 21 Aug 2007 17:12
Posts: 28
Well, just to give some in-sight as to what version of Tribes I'm using. I'm using Lasthope with a modification to remove the version check on a player's connect. Though I'd rather have it call to a scripted function like onClientAttemptConnect and reject the player based on the return value. I honestly don't know how to reproduce any crash on my server. I've done so many scripted patches for crashes that I am aware of. Though there is another I know how to reproduce that I have been unable to patch. Its a vehicle based crash, using the oddity OS in Annihilation.

I'm going to continuously run OllyDbg on my server to see what causes crashes, I'll post .bmps of the crashes since I do not know how to patch them myself.

I was wondering if you had lpatches for the buffer overflow crash of echo, export, and anything else. It would really be appreciated.


Top
 Profile  
 
 Post subject:
PostPosted: 15 Oct 2007 02:55 

Joined: 21 Aug 2007 17:12
Posts: 28
http://kigen.ath.cx/crash10142007.bmp

Yet another crash, it seems to be another pointer that is pointing to a NULL if I am reading that correctly.

At the time, from the logs, what happened was the mission just started on D R O P Z O N E_2 0 0 5 and both team's generators get destroyed at almost the same time.


Top
 Profile  
 
 Post subject:
PostPosted: 15 Oct 2007 08:51 

Joined: 13 Aug 2007 21:44
Posts: 4068
Location: http://aluigi.org
Isn't better to use PNG instead of a bitmap of over 2 megabytes?


Top
 Profile  
 
 Post subject:
PostPosted: 15 Oct 2007 09:29 

Joined: 21 Aug 2007 17:12
Posts: 28
I'm just directly using Print Screen.

Me is lazy. :lol:


Top
 Profile  
 
 Post subject:
PostPosted: 21 Oct 2007 00:47 

Joined: 21 Aug 2007 17:12
Posts: 28
Andrew hates me. :cry:

*cries and rolls around on the floor awkwardly*


Top
 Profile  
 
 Post subject:
PostPosted: 23 Oct 2007 01:51 

Joined: 10 Sep 2007 05:40
Posts: 6
Have you tried setting a breakpoint on that function and seeing what's going on? You aren't going to learn anything if I just tell you what's crashing and give you a patch.

You should be able to tell what the function is doing just by scrolling up a bit..


Top
 Profile  
 
 Post subject:
PostPosted: 28 Oct 2007 23:03 

Joined: 21 Aug 2007 17:12
Posts: 28
Unfortunately, I haven't had the time to play with ASM. But I need stable servers. :P

A lot of people got tired of the servers crashes. But still, a lot of people stayed.

Plasmatic could also use these patches as well.


Top
 Profile  
 
Display posts from previous:  Sort by  
Forum locked This topic is locked, you cannot edit posts or make further replies.  [ 15 posts ] 

All times are UTC [ DST ]


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for: