Closed rcvalle closed 10 years ago
It was not inadvertent, the changes to the source for single_reverse_tcp_shell.asm are reflected there. However, I think you're right that my changes were not intended to replace shellcode generated from unixasm.
All of our shellcode source needs some serious cleanup to avoid things like this in the future. We need to:
The single_reverse_tcp_shell.asm is the source for modules/payloads/singles/linux/x86/shell/reverse_tcp.rb , not for modules/payloads/singles/linux/x86/shell_reverse_tcp.rb; the latter is from unixasm.
Regardless, I'm creating a new repository to maintain and add new shellcodes, with a mechanism to generate both C header files and payload modules from the shellcodes. So, what I can do is to also have them as modules in the repository or have them in a separate repository so you can add them as a submodule.
Let me know if it helps or if I can help with anything else.
Additionally, there are several arbitrary changes made to my shellcodes that breaks them. For instance, https://github.com/rapid7/metasploit-framework/commit/f38ac954b8fb9d4f1106ccbf11592a613908fde7#diff-61b9630771bed89aeb55761ee5e6e16bL36 changes a pushl $0x66/popl %eax sequence for a movb $0x66,%al, but without previously clearing the high part of the %eax or %eax at all, and that was the reason for the pushl/popl sequence, it pushes a dword/long on the stack and clear the high part of %eax upon poping it with fewer bytes than doing it in any other way in IA-32 (e.g., 3 against 4 bytes in a classic xorl/movb combination).
I just saw you also added that mul at https://github.com/rapid7/metasploit-framework/commit/f38ac954b8fb9d4f1106ccbf11592a613908fde7#diff-61b9630771bed89aeb55761ee5e6e16bR34, which clears %eax and %edx, but that is still 4 against 3 bytes in the pushl/popl sequence.
modules/payloads/singles/linux/x86/shell/reverse_tcp.rb
does not exist.
Right, eax and edx are zeroed after xor ebx,ebx; mul ebx
. Without the mul
, you still need to clear edx. Previously, this was done with a pop that abused the fact that a NULL was on the stack as an argument to a prior syscall. In retrospect, a cdq
would probably be clearer.
Sorry, it was a typo. I meant modules/payloads/stagers/linux/x86/reverse_tcp.rb.
If by previously you meant my original shellcode, no, it doesn't abuse the fact that there is any null on the stack. The pushl $0x66 pushes a dword/long on the stack, 4 bytes (i.e., 0x00000066), and when it subsequently pops from the stack, the %eax register is set to this dword/long.
The mull/movb combination is 4 bytes as in a classic xorl/movb combination, the previous pushl/popl sequence was 3 bytes.
No, i mean clearing edx was done with a pop, see line 56 of the original
And no, single_reverse_tcp_shell.asm
is not the source for modules/payloads/stagers/linux/x86/reverse_tcp.rb
. That's a stager, whose source is in stager_sock_reverse.asm
In total, the mul
trick comes out to the same number of bytes as your original version. I just looked again and we can save a byte by going back to push/pop and then using eax instead of edx as the null for execve
, since it will be 0 if connect
succeeded.
If it's not, I don't know which module the single_reverse_tcp_shell.asm is source for. It may have been copied from unixasm into a NASM source.
It may end up with the same size if you save one byte somewhere else (and you did at https://github.com/rapid7/metasploit-framework/commit/f38ac954b8fb9d4f1106ccbf11592a613908fde7#diff-61b9630771bed89aeb55761ee5e6e16bL55, but you also added a zero byte to the raw payload, not considering the IP address). However, like I said above, the mull/movb combination is 4 bytes, the pushl/popl sequence is 3 bytes. They're not the same size.
So, what are you going to do with the payloads?
They are the same size when you consider that edx was used as a NULL later in the code and it allowed removing a pop edx
instruction to set that up. Anyway, that doesn't matter.
Before unixasm was added, the last time any of those source files were touched was the initial commit from skape when he brought them in from msf2. So single_reverse_tcp_shell.asm
was the original source for payloads/singles/linux/x86/shell_reverse_tcp.rb
. When I did the commit in question, I didn't realize it had been replaced because the source was not updated.
As for what to do with the payloads now, I would dearly love for you to consolidate all of our unix shellcode into one place, with one build system
Give me a few days then. I'll post updates to this issue.
@rcvalle: How's it going?
@wvu-r7 I'm still working on it. It's taking longer due to time constraints. Initially, it is very likely it will contain only the payloads from unixasm, and later I will add stagers from those and payloads for other operating systems and architectures. The payload modules are going to be generated from a make target. Is there anything else you would like to see implemented?
I'm closing this since this work is now being done in metasploit-payloads repository.
I was doing some stuff today with msfpayload and noticed the output of one of my shellcodes was different than the original, then I checked its module and found out it was replaced in this commit: https://github.com/rapid7/metasploit-framework/commit/f38ac954b8fb9d4f1106ccbf11592a613908fde7#diff-bd4a8d0d7fd28f30b92633ebd85dd6a8. Was it intentional? Because the commit message mentions stagers, and it isn't actually a stager.
None of the modified source codes corresponds to that payload, its source is in external/source/unixasm along with the others single payloads' sources. It seems that the stagers' sources were modified and that payload was inadvertently replaced along with the stagers' modules.