rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
34.04k stars 13.94k forks source link

Bug in the assembly of the meterpreter x64 stager #5988

Closed kazabubu21 closed 9 years ago

kazabubu21 commented 9 years ago

The bug is in the file: metasploit-framework/external/source/shellcode/windows/x64/src/block/block_recv.asm

and it effect on almost all payloads that were generated by msf for using on x64. The bug is this: The code call "recv" function with the parameter "4" for the received content length. The size of the register rsi is 8 byte, so when you pop to rsi(line 26 in the code file), 8 bytes where taken from the stack. Because you pushed only 4 bytes, the other 4 bytes can be anything else. It can be 00000000 and than all will work fine, and it can be another thing, and than the computer will wait to stagger in a bigger size from the real size. In this case, the handler of ms will send the stagger and than will stuck, because the victim computer will wait to some more bytes of code.

As a fix, we can offer some things: -Set the size of the returned buffer to 8 bytes (dword *2); -Use esi instead of using rsi in all places in the code(lines 26,31,40 in the original .asm file). It will also make the shell code shorter in 3 bytes. -Adding "mov esi,esi" after "pop rsi". It will reset the first 4 bytes of rsi to 0.

Here is the code from git:

https://github.com/rapid7/metasploit-framework/blob/master/external/source/shellcode/windows/x64/src/block/block_recv.asm

And finally,we want to thank you for the great framework and hope you will fix it as soon as you can. Thank you very much.

OJ commented 9 years ago

Hello @kazabubu21,

Thanks for the report. We actually stumbled on this issue quite recently ourselves! While doing some work on stager resiliency, and moving payloads over to the lib folder where metasm compiles them on the fly, @bcook-r7 found the exact same problem.

The new stagers have been fixed, as per this commit: https://github.com/rapid7/metasploit-framework/commit/9767de9bd0a0bb168002ea4e51e0ea929c23873c

We're still in the process of moving all of the payloads over to metasm, but I'm not sure that there are any x64 payloads left over that still use the old shellcode source. This is something that we need to look into.

I will leave this issue open so that we can make a point of checking all the payloads to make sure that they have been moved over.

Thanks again for the contribution!

bcook-r7 commented 9 years ago

We should definitely fix this. Question is if we should go ahead and do the metasm port now, or just hobble along and regenerate the payloads as-is.

bcook-r7 commented 9 years ago

I think we should probably do the second thing first, then metasm port over time.

OJ commented 9 years ago

Yup, I think you're right.

OJ commented 9 years ago

Do we have any idea how many payloads are affected by this?

bcook-r7 commented 9 years ago

It looks like bind_tcp still needs to be fixed, but is it possible that all the other 64-bit stagers have already been converted to metasm? I couldn't find an actual 64-bit nonx variant. Perhaps we should remove any converted .asm files to avoid confusion.

OJ commented 9 years ago

Ah right, we missed the bind one. Yeah I think the rest are close to done. I'll check.

jvazquez-r7 commented 9 years ago

Thanks @kazabubu21 for noticing it and all the feedback, both 64 bits stagers are being updated on #6016 to use the mov esi, esi path pointed by @kazabubu21 (good one! thank you!)

6016 also updated block_recv.asm to have it up to date as long as it lives there.