From 5f8181efa7c2eb1a8d2d32b0adad26fff596cf29 Mon Sep 17 00:00:00 2001 From: Adam Cammack Date: Thu, 7 May 2020 10:58:03 -0500 Subject: [PATCH] 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 | 15 +++++++++++++- lib/msf/core/payload_generator.rb | 16 +++++++++++--- spec/lib/msf/core/encoded_payload_spec.rb | 16 ++++++++++---- spec/lib/msf/core/payload_generator_spec.rb | 23 ++++++++++++++------- 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/lib/msf/core/encoded_payload.rb b/lib/msf/core/encoded_payload.rb index f07fc5ca73..4c734156f6 100644 --- a/lib/msf/core/encoded_payload.rb +++ b/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 diff --git a/lib/msf/core/payload_generator.rb b/lib/msf/core/payload_generator.rb index 7c3a3b4ab2..bf105982a2 100644 --- a/lib/msf/core/payload_generator.rb +++ b/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] 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) diff --git a/spec/lib/msf/core/encoded_payload_spec.rb b/spec/lib/msf/core/encoded_payload_spec.rb index 18bfe8bdb3..a0ee4886ff 100644 --- a/spec/lib/msf/core/encoded_payload_spec.rb +++ b/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 diff --git a/spec/lib/msf/core/payload_generator_spec.rb b/spec/lib/msf/core/payload_generator_spec.rb index df6b5997a7..3ac7b277ba 100644 --- a/spec/lib/msf/core/payload_generator_spec.rb +++ b/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