Land #3770, resolve random stager bugs

lib/msf/core/encoded_payload.rb

@@ -158,6 +158,18 @@ class EncodedPayload
           next
         end
 
+        # If the caller explictly requires register preservation, make sure
+        # that the module in question can handle it. This is mostly used by
+        # the stage encoder path.
+        if (reqs['ForceSaveRegisters'] and
+            reqs['EncoderOptions'] and
+            (reqs['EncoderOptions']['SaveRegisters'].to_s.length > 0) and
+            (! self.encoder.preserves_registers?))
+          wlog("#{pinst.refname}: Encoder #{encoder.refname} does not preserve registers and the caller needs #{reqs['EncoderOptions']['SaveRegisters']} preserved.",
+            'core', LEV_1)
+          next
+        end
+
         # Import the datastore from payload (and likely exploit by proxy)
         self.encoder.share_datastore(pinst.datastore)
 
@@ -224,12 +236,10 @@ class EncodedPayload
         self.encoded = eout
         break
       }
-
       # If the encoded payload is nil, raise an exception saying that we
       # suck at life.
       if (self.encoded == nil)
         self.encoder = nil
-
         raise NoEncodersSucceededError,
           "#{pinst.refname}: All encoders failed to encode.",
           caller

lib/msf/core/encoder.rb

@@ -413,6 +413,27 @@ class Encoder < Module
     buf
   end
 
+  #
+  # Determines whether the encoder can preserve registers at all
+  #
+  def preserves_registers?
+    false
+  end
+
+  #
+  # A list of registers always modified by the encoder
+  #
+  def modified_registers
+    []
+  end
+
+  #
+  # Determines whether the encoder can preserve the stack frame
+  #
+  def preserves_stack?
+    false
+  end
+
 protected
 
   #

lib/msf/core/payload/stager.rb

@@ -16,6 +16,8 @@ module Msf::Payload::Stager
       [
         Msf::OptBool.new("EnableStageEncoding", [ false, "Encode the second stage payload", false ]),
         Msf::OptString.new("StageEncoder", [ false, "Encoder to use if EnableStageEncoding is set", nil ]),
+        Msf::OptString.new("StageEncoderSaveRegisters", [ false, "Additional registers to preserve in the staged payload if EnableStageEncoding is set", "" ]),
+        Msf::OptBool.new("StageEncodingFallback", [ false, "Fallback to default encoders or no encoding if the selected StageEncoder is not compatible", true ])
       ], Msf::Payload::Stager)
 
   end
@@ -92,14 +94,12 @@ module Msf::Payload::Stager
     true
   end
 
-
   #
   # Whether to use an Encoder on the second stage
   #
   # @return [Boolean]
   def encode_stage?
-    # Convert to string in case it hasn't been normalized
-    !!(datastore['EnableStageEncoding'].to_s == "true")
+    !!(datastore['EnableStageEncoding'])
   end
 
   #
@@ -134,7 +134,18 @@ module Msf::Payload::Stager
       p = generate_stage
 
       # Encode the stage if stage encoding is enabled
-      p = encode_stage(p)
+      begin
+        p = encode_stage(p)
+      rescue ::RuntimeError
+        warning_msg = "Failed to stage"
+        warning_msg << " (#{conn.peerhost})"  if conn.respond_to? :peerhost
+        warning_msg << ": #{$!}"
+        print_warning warning_msg
+        if conn.respond_to? :close && !conn.closed?
+          conn.close
+        end
+        return
+      end
 
       # Give derived classes an opportunity to an intermediate state before
       # the stage is sent.  This gives derived classes an opportunity to
@@ -196,30 +207,75 @@ module Msf::Payload::Stager
     false
   end
 
+  #
+  # Takes an educated guess at the list of registers an encoded stage
+  # would need to preserve based on the Convention
+  #
+  def encode_stage_preserved_registers
+    module_info['Convention'].to_s.scan(/\bsock([a-z]{3,}+)\b/).
+      map {|reg| reg.first }.
+      join(" ")
+  end
+
   # Encodes the stage prior to transmission
   # @return [String] Encoded version of +stg+
   def encode_stage(stg)
     return stg unless encode_stage?
+    stage_enc_mod = []
 
-    if datastore["StageEncoder"].nil? or datastore["StageEncoder"].empty?
-      stage_enc_mod = nil
-    else
-      stage_enc_mod = datastore["StageEncoder"]
+    # Handle StageEncoder if specified by the user
+    if datastore['StageEncoder'].to_s.length > 0
+      # Allow multiple encoders separated by commas
+      stage_enc_mod = datastore["StageEncoder"].split(',').map(&:strip).select{|x| x.to_s.length > 0}.uniq
     end
 
-    # Generate an encoded version of the stage.  We tell the encoding system
-    # to save edi to ensure that it does not get clobbered.
-    encp = Msf::EncodedPayload.create(
-      self,
-      'Raw'           => stg,
-      'Encoder'       => stage_enc_mod,
-      'SaveRegisters' => ['edi'],
-      'ForceEncode'   => true)
-    print_status("Encoded stage with #{encp.encoder.refname}")
+    # Add automatic encoding as a fallback if needed
+    if datastore['StageEncodingFallback']
+      stage_enc_mod << nil
+    end
 
-    # If the encoding succeeded, use the encoded buffer.  Otherwise, fall
-    # back to using the non-encoded stage
-    encp.encoded || stg
+    # If fallback has been disabled and no encoder was parsed, exit early and rop the session
+    if stage_enc_mod.length == 0
+      raise RuntimeError, "StageEncoder is invalid and StageEncodingFallback is disabled"
+    end
+
+    # Allow the user to specify additional registers to preserve
+    saved_registers = (
+      datastore['StageEncoderSaveRegisters'].to_s + " "
+      encode_stage_preserved_registers
+    ).strip
+
+    estg = nil
+
+    stage_enc_mod.each do |encoder_refname_from_user|
+
+      # Generate an encoded version of the stage.  We tell the encoding system
+      # to save certain registers to ensure that it does not get clobbered.
+      encp = Msf::EncodedPayload.create(
+        self,
+        'Raw'                => stg,
+        'Encoder'            => encoder_refname_from_user,
+        'EncoderOptions'     => { 'SaveRegisters' => saved_registers },
+        'ForceSaveRegisters' => true,
+        'ForceEncode'        => true)
+
+      if encp.encoder
+        print_status("Encoded stage with #{encp.encoder.refname}")
+        estg = encp.encoded
+        break
+      end
+    end
+
+    if datastore['StageEncodingFallback'] && estg.nil?
+      print_warning("StageEncoder failed, falling back to no encoding")
+      estg = stg
+    end
+
+    unless estg
+      raise RuntimeError, "Stage encoding failed and StageEncodingFallback is disabled"
+    end
+
+    estg
   end
 
   # Aliases

lib/rex/arch/x86.rb

@@ -520,6 +520,22 @@ module X86
     return nil
   end
 
+  #
+  # Parse a list of registers as a space or command delimited
+  # string and return the internal register IDs as an array
+  #
+  def self.register_names_to_ids(str)
+    register_ids = []
+    str.to_s.strip.split(/[,\s]/).
+      map    {|reg| reg.to_s.strip.upcase }.
+      select {|reg| reg.length > 0        }.
+      uniq.each do |reg|
+        next unless self.const_defined?(reg.intern)
+        register_ids << self.const_get(reg.intern)
+      end
+    register_ids
+  end
+
 end
 
 end end

modules/encoders/x86/call4_dword_xor.rb

@@ -28,6 +28,12 @@ class Metasploit3 < Msf::Encoder::Xor
   # the buffer being encoded
   #
   def decoder_stub(state)
+
+    # Sanity check that saved_registers doesn't overlap with modified_registers
+    if (modified_registers & saved_registers).length > 0
+      raise BadGenerateError
+    end
+
     decoder =
       Rex::Arch::X86.sub(-(((state.buf.length - 1) / 4) + 1), Rex::Arch::X86::ECX,
         state.badchars) +
@@ -44,4 +50,19 @@ class Metasploit3 < Msf::Encoder::Xor
     return decoder
   end
 
+  # Indicate that this module can preserve some registers
+  def preserves_registers?
+    true
+  end
+
+  # A list of registers always touched by this encoder
+  def modified_registers
+    [ Rex::Arch::X86::ECX, Rex::Arch::X86::EAX, Rex::Arch::X86::ESI ]
+  end
+
+  # Convert the SaveRegisters to an array of x86 register constants
+  def saved_registers
+    Rex::Arch::X86.register_names_to_ids(datastore['SaveRegisters'])
+  end
+
 end

modules/encoders/x86/countdown.rb

@@ -30,16 +30,22 @@ class Metasploit3 < Msf::Encoder::Xor
   # being encoded.
   #
   def decoder_stub(state)
+
+    # Sanity check that saved_registers doesn't overlap with modified_registers
+    if (modified_registers & saved_registers).length > 0
+      raise BadGenerateError
+    end
+
     decoder =
       Rex::Arch::X86.set(
         Rex::Arch::X86::ECX,
         state.buf.length - 1,
         state.badchars) +
-      "\xe8\xff\xff\xff" +		# call $+4
-      "\xff\xc1" +			# inc ecx
-      "\x5e" +			# pop esi
-      "\x30\x4c\x0e\x07" +		# xor_loop: xor [esi + ecx + 0x07], cl
-      "\xe2\xfa"			# loop xor_loop
+      "\xe8\xff\xff\xff" +  # call $+4
+      "\xff\xc1" +          # inc ecx
+      "\x5e" +              # pop esi
+      "\x30\x4c\x0e\x07" +  # xor_loop: xor [esi + ecx + 0x07], cl
+      "\xe2\xfa"            # loop xor_loop
 
     # Initialize the state context to 1
     state.context = 1
@@ -57,4 +63,18 @@ class Metasploit3 < Msf::Encoder::Xor
     [ block.unpack('C')[0] ^ (state.context - 1) ].pack('C')
   end
 
+  # Indicate that this module can preserve some registers
+  def preserves_registers?
+    true
+  end
+
+  # A list of registers always touched by this encoder
+  def modified_registers
+    [ Rex::Arch::X86::ECX, Rex::Arch::X86::ESI ]
+  end
+
+  # Convert the SaveRegisters to an array of x86 register constants
+  def saved_registers
+    Rex::Arch::X86.register_names_to_ids(datastore['SaveRegisters'])
+  end
 end

modules/encoders/x86/fnstenv_mov.rb

@@ -31,6 +31,12 @@ class Metasploit3 < Msf::Encoder::Xor
   # being encoded.
   #
   def decoder_stub(state)
+
+    # Sanity check that saved_registers doesn't overlap with modified_registers
+    if (modified_registers & saved_registers).length > 0
+      raise BadGenerateError
+    end
+
     decoder =
       Rex::Arch::X86.set(
         Rex::Arch::X86::ECX,
@@ -48,4 +54,18 @@ class Metasploit3 < Msf::Encoder::Xor
     return decoder
   end
 
+  # Indicate that this module can preserve some registers
+  def preserves_registers?
+    true
+  end
+
+  # A list of registers always touched by this encoder
+  def modified_registers
+    [ Rex::Arch::X86::EBX, Rex::Arch::X86::ECX ]
+  end
+
+  # Convert the SaveRegisters to an array of x86 register constants
+  def saved_registers
+    Rex::Arch::X86.register_names_to_ids(datastore['SaveRegisters'])
+  end
 end

modules/encoders/x86/shikata_ga_nai.rb

@@ -37,9 +37,16 @@ class Metasploit3 < Msf::Encoder::XorAdditiveFeedback
   # Generates the shikata decoder stub.
   #
   def decoder_stub(state)
+
     # If the decoder stub has not already been generated for this state, do
     # it now.  The decoder stub method may be called more than once.
     if (state.decoder_stub == nil)
+
+      # Sanity check that saved_registers doesn't overlap with modified_registers
+      if (modified_registers & saved_registers).length > 0
+        raise BadGenerateError
+      end
+
       # Shikata will only cut off the last 1-4 bytes of it's own end
       # depending on the alignment of the original buffer
       cutoff = 4 - (state.buf.length & 3)
@@ -61,6 +68,27 @@ class Metasploit3 < Msf::Encoder::XorAdditiveFeedback
     state.decoder_stub
   end
 
+  # Indicate that this module can preserve some registers
+  def preserves_registers?
+    true
+  end
+
+  # A list of registers always touched by this encoder
+  def modified_registers
+    # ESP is assumed and is handled through preserves_stack?
+    [
+      # The counter register is hardcoded
+      Rex::Arch::X86::ECX,
+      # These are modified by div and mul operations
+      Rex::Arch::X86::EAX, Rex::Arch::X86::EDX
+    ]
+  end
+
+  # Always blacklist these registers in our block generation
+  def block_generator_register_blacklist
+    [Rex::Arch::X86::ESP, Rex::Arch::X86::ECX] | saved_registers
+  end
+
 protected
 
   #
@@ -251,15 +279,18 @@ protected
     loop_inst.depends_on(loop_block)
 
     begin
-      # Generate a permutation saving the ECX and ESP registers
-      loop_inst.generate([
-        Rex::Arch::X86::ESP,
-        Rex::Arch::X86::ECX ], nil, state.badchars)
+      # Generate a permutation saving the ECX, ESP, and user defined registers
+      loop_inst.generate(block_generator_register_blacklist, nil, state.badchars)
     rescue RuntimeError => e
       raise EncodingError
     end
   end
 
+  # Convert the SaveRegisters to an array of x86 register constants
+  def saved_registers
+    Rex::Arch::X86.register_names_to_ids(datastore['SaveRegisters'])
+  end
+
   def sub_immediate(regnum, imm)
     return "" if imm.nil? or imm == 0
     if imm > 255 or imm < -255