Avoid auto-encoding payloads free of badchars

Payloads without any of the specified badchars will no longer be encoded
by default. This should hopefully lead to less surprising results when
using simple payloads (especially commands. Things that had incomplete
badchar analysis may break as a result, since not everything will be
encoded by default anymore. Sorry in advance if they do.

lib/msf/core/encoded_payload.rb

@@ -127,7 +127,7 @@ class EncodedPayload
   def encode
     # If the exploit has bad characters, we need to run the list of encoders
     # in ranked precedence and try to encode without them.
-    if reqs['BadChars'].to_s.length > 0 or reqs['Encoder'] or reqs['ForceEncode']
+    if reqs['Encoder'] || reqs['ForceEncode'] || has_chars?(reqs['BadChars'])
       encoders = pinst.compatible_encoders
 
       # Make sure the encoder name from the user has the same String#encoding
@@ -276,6 +276,10 @@ class EncodedPayload
     # If there are no bad characters, then the raw is the same as the
     # encoded
     else
+      unless reqs['BadChars'].blank?
+        ilog("#{pinst.refname}: payload contains no badchars, skipping automatic encoding", 'core', LEV_0)
+      end
+
       self.encoded = raw
     end
 
@@ -512,6 +516,15 @@ protected
   #
   attr_accessor :reqs
 
+  def has_chars?(chars)
+    return false if chars.blank? || self.raw.blank?
+
+    chars.each_byte do |bad|
+      return true if self.raw.index(bad.chr(Encoding::ASCII_8BIT))
+    end
+
+    false
+  end
 end
 
 end

lib/msf/core/payload_generator.rb

@@ -257,9 +257,9 @@ module Msf
     # @return [String] The encoded shellcode
     def encode_payload(shellcode)
       shellcode = shellcode.dup
-      encoder_list = get_encoders
+      encoder_list = get_encoders(shellcode)
       if encoder_list.empty?
-        cli_print "No encoder or badchars specified, outputting raw payload"
+        cli_print "No encoder specified, outputting raw payload"
         return shellcode
       end
 
@@ -467,7 +467,7 @@ module Msf
     # This method returns an array of encoders that either match the
     # encoders selected by the user, or match the arch selected.
     # @return [Array<Msf::Encoder>] An array of potential encoders to use
-    def get_encoders
+    def get_encoders(buf)
       encoders = []
       if encoder.present?
         # Allow comma separated list of encoders so users can choose several
@@ -486,6 +486,16 @@ module Msf
         end
         encoders.sort_by { |my_encoder| my_encoder.rank }.reverse
       elsif !badchars.empty? && !badchars.nil?
+        badchars_present = false
+        badchars.each_byte do |bad|
+          badchars_present = true if buf.index(bad.chr(Encoding::ASCII_8BIT))
+        end
+
+        unless badchars_present
+          cli_print "No badchars present in payload, skipping automatic encoding"
+          return []
+        end
+
         framework.encoders.each_module_ranked('Arch' => [arch], 'Platform' => platform_list) do |name, mod|
           e = framework.encoders.create(name)
           e.datastore.import_options_from_hash(datastore)

spec/lib/msf/core/encoded_payload_spec.rb

@@ -121,12 +121,20 @@ RSpec.describe Msf::EncodedPayload do
     context 'with bad characters: "\\0"' do
       let(:badchars) { "\0".force_encoding('binary') }
 
-      specify 'chooses x86/shikata_ga_nai' do
-        expect(encoded_payload.encoder.refname).to eq("x86/shikata_ga_nai")
+      context 'when the payload contains the bad characters' do
+        specify 'chooses x86/shikata_ga_nai' do
+          expect(encoded_payload.encoder.refname).to eq("x86/shikata_ga_nai")
+        end
+
+        specify do
+          expect(encoded_payload.encoded).not_to include(badchars)
+        end
       end
 
-      specify do
-        expect(encoded_payload.encoded).not_to include(badchars)
+      context 'when the payload does not contain the bad characters' do
+        specify 'returns the raw value' do
+          expect(encoded_payload.generate("RAW")).to eql("RAW")
+        end
       end
 
     end

spec/lib/msf/core/payload_generator_spec.rb

@@ -832,17 +832,18 @@ RSpec.describe Msf::PayloadGenerator do
           template: File.join(Msf::Config.data_directory, 'templates', 'template_x86_windows.exe')
       }
     }
+    let(:shellcode) { "a test payload" }
     context 'when an encoder is selected' do
       it 'returns an array' do
-        expect(payload_generator.get_encoders).to be_kind_of Array
+        expect(payload_generator.get_encoders(shellcode)).to be_kind_of Array
       end
 
       it 'returns an array with only one element' do
-        expect(payload_generator.get_encoders.count).to eq 1
+        expect(payload_generator.get_encoders(shellcode).count).to eq 1
       end
 
       it 'returns the correct encoder in the array' do
-        expect(payload_generator.get_encoders.first.name).to eq encoder_names[0]
+        expect(payload_generator.get_encoders(shellcode).first.name).to eq encoder_names[0]
       end
     end
 
@@ -890,17 +891,17 @@ RSpec.describe Msf::PayloadGenerator do
       end
 
       it 'returns an array of the right size' do
-        expect(payload_generator.get_encoders.count).to eq 2
+        expect(payload_generator.get_encoders(shellcode).count).to eq 2
       end
 
       it 'returns each of the selected encoders in the array' do
-        payload_generator.get_encoders.each do |msf_encoder|
+        payload_generator.get_encoders(shellcode).each do |msf_encoder|
           expect(encoder_names).to include msf_encoder.name
         end
       end
 
       it 'returns the encoders in order of rank high to low' do
-        expect(payload_generator.get_encoders[0].rank).to be > payload_generator.get_encoders[1].rank
+        expect(payload_generator.get_encoders(shellcode)[0].rank).to be > payload_generator.get_encoders(shellcode)[1].rank
       end
     end
 
@@ -927,10 +928,16 @@ RSpec.describe Msf::PayloadGenerator do
       }
 
       it 'returns an array of all encoders with a compatible arch' do
-        payload_generator.get_encoders.each do |my_encoder|
+        payload_generator.get_encoders(shellcode).each do |my_encoder|
           expect(my_encoder.arch).to include 'x86'
         end
       end
+      context 'when badchars are specified but are not present in the payload' do
+        let(:shellcode) { "nobadchars" }
+        it 'returns an empty array' do
+          expect(payload_generator.get_encoders(shellcode)).to be_empty
+        end
+      end
     end
 
     context 'when no encoder or badchars are selected' do
@@ -961,7 +968,7 @@ RSpec.describe Msf::PayloadGenerator do
         }
 
       it 'returns an empty array' do
-        expect(payload_generator.get_encoders).to be_empty
+        expect(payload_generator.get_encoders(shellcode)).to be_empty
       end
     end
   end