Land rapid7#4479, nil comparisons and missing DLLs

Also fixes rapid7#4474.

Author: Tod Beardsley

lib/rex/post/meterpreter/client_core.rb

@@ -68,33 +68,33 @@ def load_library(opts)
     load_flags   = LOAD_LIBRARY_FLAG_LOCAL
 
     # No library path, no cookie.
-    if (library_path == nil)
+    if library_path.nil?
       raise ArgumentError, "No library file path was supplied", caller
     end
 
     # Set up the proper loading flags
-    if (opts['UploadLibrary'])
+    if opts['UploadLibrary']
       load_flags &= ~LOAD_LIBRARY_FLAG_LOCAL
     end
-    if (opts['SaveToDisk'])
+    if opts['SaveToDisk']
       load_flags |= LOAD_LIBRARY_FLAG_ON_DISK
     end
-    if (opts['Extension'])
+    if opts['Extension']
       load_flags |= LOAD_LIBRARY_FLAG_EXTENSION
     end
 
     # Create a request packet
     request = Packet.create_request('core_loadlib')
 
     # If we must upload the library, do so now
-    if ((load_flags & LOAD_LIBRARY_FLAG_LOCAL) != LOAD_LIBRARY_FLAG_LOCAL)
+    if (load_flags & LOAD_LIBRARY_FLAG_LOCAL) != LOAD_LIBRARY_FLAG_LOCAL
       image = ''
 
       ::File.open(library_path, 'rb') { |f|
         image = f.read
       }
 
-      if (image != nil)
+      if !image.nil?
         request.add_tlv(TLV_TYPE_DATA, image, false, client.capabilities[:zlib])
       else
         raise RuntimeError, "Failed to serialize library #{library_path}.", caller
@@ -103,7 +103,7 @@ def load_library(opts)
       # If it's an extension we're dealing with, rename the library
       # path of the local and target so that it gets loaded with a random
       # name
-      if (opts['Extension'])
+      if opts['Extension']
         library_path = "ext" + rand(1000000).to_s + ".#{client.binary_suffix}"
         target_path  = library_path
       end
@@ -113,17 +113,17 @@ def load_library(opts)
     request.add_tlv(TLV_TYPE_LIBRARY_PATH, library_path)
     request.add_tlv(TLV_TYPE_FLAGS, load_flags)
 
-    if (target_path != nil)
+    if !target_path.nil?
       request.add_tlv(TLV_TYPE_TARGET_PATH, target_path)
     end
 
     # Transmit the request and wait the default timeout seconds for a response
     response = self.client.send_packet_wait_response(request, self.client.response_timeout)
 
     # No response?
-    if (response == nil)
+    if response.nil?
       raise RuntimeError, "No response was received to the core_loadlib request.", caller
-    elsif (response.result != 0)
+    elsif response.result != 0
       raise RuntimeError, "The core_loadlib request failed with result: #{response.result}.", caller
     end
 
@@ -147,18 +147,22 @@ def load_library(opts)
   #		memory on the remote machine
   #
   def use(mod, opts = { })
-    if (mod == nil)
+    if mod.nil?
       raise RuntimeError, "No modules were specified", caller
     end
     # Get us to the installation root and then into data/meterpreter, where
     # the file is expected to be
     modname = "ext_server_#{mod.downcase}"
     path = MeterpreterBinaries.path(modname, client.binary_suffix)
 
-    if (opts['ExtensionPath'])
+    if opts['ExtensionPath']
       path = opts['ExtensionPath']
     end
 
+    if path.nil?
+      raise RuntimeError, "No module of the name #{modname}.#{client.binary_suffix} found", caller
+    end
+
     path = ::File.expand_path(path)
 
     # Load the extension DLL
@@ -187,24 +191,24 @@ def migrate( pid )
 
     # Determine the architecture for the pid we are going to migrate into...
     client.sys.process.processes.each { | p |
-      if( p['pid'] == pid )
+      if p['pid'] == pid
         process = p
         break
       end
     }
 
     # We cant migrate into a process that does not exist.
-    if( process == nil )
+    if process.nil?
       raise RuntimeError, "Cannot migrate into non existent process", caller
     end
 
     # We cant migrate into a process that we are unable to open
-    if( process['arch'] == nil or process['arch'].empty? )
+    if process['arch'].nil? or process['arch'].empty?
       raise RuntimeError, "Cannot migrate into this process (insufficient privileges)", caller
     end
 
     # And we also cant migrate into our own current process...
-    if( process['pid'] == client.sys.process.getpid )
+    if process['pid'] == client.sys.process.getpid
       raise RuntimeError, "Cannot migrate into current process", caller
     end
 
@@ -213,10 +217,10 @@ def migrate( pid )
     c.include( ::Msf::Payload::Stager )
 
     # Include the appropriate reflective dll injection module for the target process architecture...
-    if( process['arch'] == ARCH_X86 )
+    if process['arch'] == ARCH_X86
       c.include( ::Msf::Payload::Windows::ReflectiveDllInject )
       binary_suffix = "x86.dll"
-    elsif( process['arch'] == ARCH_X86_64 )
+    elsif process['arch'] == ARCH_X86_64
       c.include( ::Msf::Payload::Windows::ReflectiveDllInject_x64 )
       binary_suffix = "x64.dll"
     else
@@ -225,7 +229,12 @@ def migrate( pid )
 
     # Create the migrate stager
     migrate_stager = c.new()
-    migrate_stager.datastore['DLL'] = MeterpreterBinaries.path('metsrv',binary_suffix)
+
+    dll = MeterpreterBinaries.path('metsrv',binary_suffix)
+    if dll.nil?
+      raise RuntimeError, "metsrv.#{binary_suffix} not found", caller
+    end
+    migrate_stager.datastore['DLL'] = dll
 
     blob = migrate_stager.stage_payload
 
@@ -253,7 +262,7 @@ def migrate( pid )
     request.add_tlv( TLV_TYPE_MIGRATE_PID, pid )
     request.add_tlv( TLV_TYPE_MIGRATE_LEN, blob.length )
     request.add_tlv( TLV_TYPE_MIGRATE_PAYLOAD, blob, false, client.capabilities[:zlib])
-    if( process['arch'] == ARCH_X86_64 )
+    if process['arch'] == ARCH_X86_64
       request.add_tlv( TLV_TYPE_MIGRATE_ARCH, 2 ) # PROCESS_ARCH_X64
     else
       request.add_tlv( TLV_TYPE_MIGRATE_ARCH, 1 ) # PROCESS_ARCH_X86
@@ -301,7 +310,7 @@ def migrate( pid )
     # Update the meterpreter platform/suffix for loading extensions as we may have changed target architecture
     # sf: this is kinda hacky but it works. As ruby doesnt let you un-include a module this is the simplest solution I could think of.
     # If the platform specific modules Meterpreter_x64_Win/Meterpreter_x86_Win change significantly we will need a better way to do this.
-    if( process['arch'] == ARCH_X86_64 )
+    if process['arch'] == ARCH_X86_64
       client.platform      = 'x64/win64'
       client.binary_suffix = 'x64.dll'
     else

lib/rex/post/meterpreter/extensions/priv/priv.rb

@@ -46,6 +46,9 @@ def getsystem( technique=0 )
     elevator_name = Rex::Text.rand_text_alpha_lower( 6 )
 
     elevator_path = MeterpreterBinaries.path('elevator', client.binary_suffix)
+    if elevator_path.nil?
+      raise RuntimeError, "elevator.#{binary_suffix} not found", caller
+    end
 
     elevator_path = ::File.expand_path( elevator_path )
 

lib/rex/post/meterpreter/extensions/stdapi/ui.rb

@@ -157,6 +157,9 @@ def screenshot( quality=50 )
     # include the x64 screenshot dll if the host OS is x64
     if( client.sys.config.sysinfo['Architecture'] =~ /^\S*x64\S*/ )
       screenshot_path = MeterpreterBinaries.path('screenshot','x64.dll')
+      if screenshot_path.nil?
+        raise RuntimeError, "screenshot.x64.dll not found", caller
+      end
       screenshot_path = ::File.expand_path( screenshot_path )
       screenshot_dll  = ''
       ::File.open( screenshot_path, 'rb' ) do |f|
@@ -165,8 +168,11 @@ def screenshot( quality=50 )
       request.add_tlv( TLV_TYPE_DESKTOP_SCREENSHOT_PE64DLL_BUFFER, screenshot_dll, false, true )
       request.add_tlv( TLV_TYPE_DESKTOP_SCREENSHOT_PE64DLL_LENGTH, screenshot_dll.length )
     end
-    # but allways include the x86 screenshot dll as we can use it for wow64 processes if we are on x64
+    # but always include the x86 screenshot dll as we can use it for wow64 processes if we are on x64
     screenshot_path = MeterpreterBinaries.path('screenshot','x86.dll')
+    if screenshot_path.nil?
+      raise RuntimeError, "screenshot.x86.dll not found", caller
+    end
     screenshot_path = ::File.expand_path( screenshot_path )
     screenshot_dll  = ''
     ::File.open( screenshot_path, 'rb' ) do |f|

spec/lib/rex/post/meterpreter/client_core_spec.rb

@@ -43,7 +43,7 @@
     context 'with a missing a module' do
       let(:mod) {"eaten_by_av"}
       it 'should be available' do
-        expect { client_core.use(mod) }.to raise_error(TypeError)
+        expect { client_core.use(mod) }.to raise_error(RuntimeError)
       end
     end