rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
34.14k stars 13.97k forks source link

#5357 breaks meterpreter sessions #5360

Closed sempervictus closed 9 years ago

sempervictus commented 9 years ago

The UUID configuration being applied by the session_block method in lib/rex/payloads/meterpreter/config.rb attempts to assign opts[:uuid].to_raw to the uuid variable in session_data. This breaks meterpreter sessions (at least here, OJ's seems to work), which die in "sending stage." Stage0 on the remote side executes, downloads a broken stage1, and happily kills itself (no lingering PIDs on the client at least).

The following diff allows payloads to work again, and shows what's going on with the UUID var (obviously dont merge this debug code please):

diff --git i/lib/rex/payloads/meterpreter/config.rb w/lib/rex/payloads/meterpreter/config.rb
index a06a19e..728c0b7 100644
--- i/lib/rex/payloads/meterpreter/config.rb
+++ w/lib/rex/payloads/meterpreter/config.rb
@@ -47,7 +47,8 @@ private
   end

   def session_block(opts)
-    uuid = opts[:uuid].to_raw
+    $stdout.write("UUID is #{opts[:uuid]}\nRAW is #{opts[:uuid].to_raw} @ #{opts[:uuid].to_raw.length} bytes\n")
+    uuid = to_str(opts[:uuid].to_raw,64) #opts[:uuid].to_raw
     exit_func = Msf::Payload::Windows.exit_types[opts[:exitfunk]]

     session_data = [

Sample output of what its producing:

[*] [2015.05.17-12:39:38] Powershell command length: 2473
[*] [2015.05.17-12:39:38] 10.1.1.20:445 - Executing the payload...
[*] [2015.05.17-12:39:38] 10.1.1.20:445 - Binding to 367abb81-9844-35f1-ad32-98f038001003:2.0@ncacn_np:10.1.1.20[\svcctl] ...
[*] [2015.05.17-12:39:39] 10.1.1.20:445 - Bound to 367abb81-9844-35f1-ad32-98f038001003:2.0@ncacn_np:10.1.1.20[\svcctl] ...
[*] [2015.05.17-12:39:39] 10.1.1.20:445 - Obtaining a service manager handle...
[*] [2015.05.17-12:39:39] 10.1.1.20:445 - Creating the service...
[+] [2015.05.17-12:39:39] 10.1.1.20:445 - Successfully created the service
[*] [2015.05.17-12:39:39] 10.1.1.20:445 - Starting the service...
[+] [2015.05.17-12:39:40] 10.1.1.20:445 - Service start timed out, OK if running a command or non-service executable...
[*] [2015.05.17-12:39:40] 10.1.1.20:445 - Removing the service...
[+] [2015.05.17-12:39:40] 10.1.1.20:445 - Successfully removed the sevice
[*] [2015.05.17-12:39:40] 10.1.1.20:445 - Closing service handle...
UUID is 91a2e3c28654fa63/x86=1/windows=1/2015-05-17T16:39:41Z
RAW is ���†T�c�֩��� @ 16 bytes
[*] [2015.05.17-12:39:41] Sending stage (884386 bytes) to 10.1.1.20
(2015-05-17)12:39 (S:0 J:4)msf  exploit(psexec_psh) > [*] Meterpreter session 1 opened (10.1.1.10:31158 -> 10.1.1.20:51207) at 2015-05-17 12:39:44 -0400

The raw uuid does appear to be 16bytes, but somewhere down the line it seems to keel. @OJ: thoughts?

This is using meterpter #162 and msf #5357

sempervictus commented 9 years ago

I may have a reason as to why this is happening: the raw uuid creates line breaks in the packed session_data.

Using the .to_raw approach:

UUID is 22777bea9721901e/x86=1/windows=1/2015-05-17T16:50:10Z
RAW is "w{��!�
�
 �_��Y @ 16 bytes
Final data: ���V�:  "w{��!�
�
 �_��Y

and then the to_str approach:

UUID is 07628cb7a43749d9/x86=1/windows=1/2015-05-17T16:51:04Z
RAW is b���7I�������! @ 16 bytes
Final data: ���V�:  b���7I�������!

The diff to reproduce this:

diff --git i/lib/rex/payloads/meterpreter/config.rb w/lib/rex/payloads/meterpreter/config.rb
index a06a19e..8102679 100644
--- i/lib/rex/payloads/meterpreter/config.rb
+++ w/lib/rex/payloads/meterpreter/config.rb
@@ -47,7 +47,9 @@ private
   end

   def session_block(opts)
-    uuid = opts[:uuid].to_raw
+    $stdout.write("UUID is #{opts[:uuid]}\nRAW is #{opts[:uuid].to_raw} @ #{opts[:uuid].to_raw.length} bytes\n")
+    uuid = to_str(opts[:uuid].to_raw,64) 
+    #uuid = opts[:uuid].to_raw
     exit_func = Msf::Payload::Windows.exit_types[opts[:exitfunk]]

     session_data = [
@@ -57,7 +59,9 @@ private
       uuid                # the UUID
     ]

-    session_data.pack("VVVA*")
+    res = session_data.pack("VVVA*")
+    $stdout.puts("Final data: #{res}\n")
+    res
   end
OJ commented 9 years ago

uuid = to_str(opts[:uuid].to_raw,64)

This is no longer valid. The UUID in Meterpreter's configuration is expected to be 16 bytes long. The extra bytes will cause issues with the rest of the packed configuration elements because they're not of the expected size. If your Meterpreter binaries are up to 0.0.7 (currently in master as of a few hours ago), then that UUID must be 16 bytes.

I can't see why line breaks, or any other printable/non-printable should cause problems here, except perhaps the packing?

res = session_data.pack("VVVA*")

Should the A* be something else for "plain bytes" ? @hmoore-r7 any insights here?

OJ commented 9 years ago

Looked at some documentation:

String       |         |
Directive    |         | Meaning
---------------------------------------------------------------------------
   A         | String  | arbitrary binary string (space padded, count is width)
   a         | String  | arbitrary binary string (null padded, count is width)
   Z         | String  | same as ``a'', except that null is added with *
   B         | String  | bit string (MSB first)
   b         | String  | bit string (LSB first)
   H         | String  | hex string (high nibble first)
   h         | String  | hex string (low nibble first)
   u         | String  | UU-encoded string
   M         | String  | quoted printable, MIME encoding (see RFC2045)
   m         | String  | base64 encoded string (see RFC 2045, count is width)
             |         | (if count is 0, no line feed are added, see RFC 4648)
   P         | String  | pointer to a structure (fixed-length string)
   p         | String  | pointer to a null-terminated string

From this table it looks like A* is fine.

@sempervictus I'm not able to reproduce your problem at all. The exact value of the 16-byte UUID shouldn't matter at all. Updating the UUID to be 64 bytes wide totally screws with things.

OJ commented 9 years ago

Chatting to @sempervictus on IRC, and did some testing locally. I set up a similar scenario using psexec_psh to see if I could repro the error on the current master.

64-bit:

msf exploit(psexec_psh) > run

[*] Started reverse handler on 10.1.10.40:4444 
[*] 10.1.10.11:445 - Executing the payload...
[+] 10.1.10.11:445 - Service start timed out, OK if running a command or non-service executable...
[*] Sending stage (1102898 bytes) to 10.1.10.11
[*] Meterpreter session 2 opened (10.1.10.40:4444 -> 10.1.10.11:49224) at 2015-05-18 11:57:39 +1000

meterpreter > uuid
[+] UUID: b2fd101e6e2daa34/x64=3/windows=1/2015-05-18T01:57:37Z
meterpreter > sysinfo
Computer        : PDC2012
OS              : Windows 2012 (Build 9200).
Architecture    : x64
System Language : en_US
Domain          : PWNAGE
Logged On Users : 5
Meterpreter     : x64/win64
meterpreter > exit

32-bit:

msf exploit(psexec_psh) > run

[*] Started reverse handler on 10.1.10.40:4444 
[*] 10.1.10.11:445 - Executing the payload...
[+] 10.1.10.11:445 - Service start timed out, OK if running a command or non-service executable...
[*] Sending stage (884270 bytes) to 10.1.10.11
[*] Meterpreter session 1 opened (10.1.10.40:4444 -> 10.1.10.11:49318) at 2015-05-18 12:12:42 +1000

meterpreter > uuid
[+] UUID: 2b3ff790609a7f30/x86=1/windows=1/2015-05-18T02:12:40Z
meterpreter > sysinfo
Computer        : PDC2012
OS              : Windows 2012 (Build 9200).
Architecture    : x64 (Current Process is WOW64)
System Language : en_US
Domain          : PWNAGE
Logged On Users : 5
Meterpreter     : x86/win32
meterpreter > exit

I can't seem to recreate the problem.

sempervictus commented 9 years ago

Worked with @OJ on this, and we have concluded that i am in fact a jackass. Stale gems with meterp binaries were still present and framework was arbitrarily using different dlls. Rebuilt fresh (with meterp head, so the last commit works), and i have sesssions showing UUIDs with the :to_raw cast from #5357.

Sorry guys, my b. Closing.

OJ commented 9 years ago

Great to hear @sempervictus ! Cheers for the update.