diff --git a/lib/msf/core/db_manager/import/metasploit_framework/zip.rb b/lib/msf/core/db_manager/import/metasploit_framework/zip.rb index fdf4a06bdc3b9..360a67b14f802 100644 --- a/lib/msf/core/db_manager/import/metasploit_framework/zip.rb +++ b/lib/msf/core/db_manager/import/metasploit_framework/zip.rb @@ -251,6 +251,14 @@ def import_msf_zip(args={}, &block) end def is_child_of?(target_dir, target) - target.downcase.start_with?(target_dir.downcase) + target_dir = File.expand_path(target_dir) + target = File.expand_path(target) + + if Gem.win_platform? + target_dir = target_dir.downcase + target = target.downcase + end + + target == target_dir || target.start_with?("#{target_dir}#{File::SEPARATOR}") end end diff --git a/lib/msf/core/rpc/v10/service.rb b/lib/msf/core/rpc/v10/service.rb index 8348abacffa23..92b60ba29cc8f 100644 --- a/lib/msf/core/rpc/v10/service.rb +++ b/lib/msf/core/rpc/v10/service.rb @@ -8,6 +8,8 @@ module RPC class Service + MAX_REQUEST_SIZE = 10 * 1024 * 1024 + attr_accessor :service, :srvhost, :srvport, :uri, :options attr_accessor :handlers, :default_handler, :tokens, :users, :framework attr_accessor :dispatcher_timeout, :token_timeout, :debug, :str_encoding @@ -57,6 +59,7 @@ def start self.options[:comm], self.options[:cert] ) + self.service.max_request_body_size = MAX_REQUEST_SIZE self.service.add_resource(self.uri, { 'Proc' => Proc.new { |cli, req| on_request_uri(cli, req) }, @@ -113,6 +116,10 @@ def process(req) raise ArgumentError, "Invalid Content Type" end + if req.body.bytesize > MAX_REQUEST_SIZE + raise ArgumentError, "RPC request body is too large (maximum #{MAX_REQUEST_SIZE} bytes)" + end + msg = MessagePack.unpack(req.body) unless (msg && msg.kind_of?(::Array) && msg.length > 0) @@ -121,7 +128,10 @@ def process(req) msg.map { |a| a.respond_to?(:force_encoding) ? a.force_encoding(self.str_encoding) : a } - group, funct = msg.shift.split(".", 2) + method_name = msg.shift + raise ArgumentError, 'Invalid API method' unless method_name.kind_of?(::String) + + group, funct = method_name.split(".", 2) unless self.handlers[group] raise ArgumentError, "Unknown API Group: '#{group.inspect}'" diff --git a/lib/rex/proto/http/packet.rb b/lib/rex/proto/http/packet.rb index 29d8837fc9129..7877cebc09892 100644 --- a/lib/rex/proto/http/packet.rb +++ b/lib/rex/proto/http/packet.rb @@ -105,6 +105,11 @@ def parse(buf, opts={}) # Continue on to the body if the header was processed if(self.state == ParseState::ProcessingBody) + max_body_size = opts[:max_body_size] + if max_body_size && !transfer_chunked && body_bytes_left > max_body_size + raise ArgumentError, "HTTP body is too large (maximum #{max_body_size} bytes)" + end + # Chunked encoding sets the parsing state on its own. # HEAD requests can return immediately. orig_method = opts.fetch(:orig_method) { '' } @@ -112,6 +117,9 @@ def parse(buf, opts={}) self.state = ParseState::Completed else parse_body + if max_body_size && body.bytesize > max_body_size + raise ArgumentError, "HTTP body is too large (maximum #{max_body_size} bytes)" + end end end rescue diff --git a/lib/rex/proto/http/server.rb b/lib/rex/proto/http/server.rb index 6dcb20a954503..953c47ef2aa46 100644 --- a/lib/rex/proto/http/server.rb +++ b/lib/rex/proto/http/server.rb @@ -14,6 +14,8 @@ module Http ### class Server + attr_accessor :max_request_body_size + include Proto # @@ -247,7 +249,7 @@ def on_client_data(cli) raise ::EOFError if not data raise ::EOFError if data.empty? - case cli.request.parse(data) + case cli.request.parse(data, max_body_size: max_request_body_size) when Packet::ParseCode::Completed dispatch_request(cli, cli.request) cli.reset_cli diff --git a/lib/snmp/mib.rb b/lib/snmp/mib.rb index 466d94c45e09c..15f6ff31a62f1 100644 --- a/lib/snmp/mib.rb +++ b/lib/snmp/mib.rb @@ -8,7 +8,9 @@ # require 'snmp/varbind' +require 'snmp/python_literal_parser' require 'fileutils' +require 'open3' require 'yaml' module SNMP @@ -24,6 +26,10 @@ class MIB MODULE_EXT = 'yaml' class ModuleNotLoadedError < RuntimeError; end + class InvalidMIBError < RuntimeError; end + + MAX_CONVERTER_OUTPUT_SIZE = 10 * 1024 * 1024 + MODULE_NAME_PATTERN = /\A[A-Za-z][A-Za-z0-9-]*\z/ class << self ## @@ -55,11 +61,18 @@ class << self def import_module(module_file, mib_dir=DEFAULT_MIB_PATH) raise "smidump tool must be installed" unless import_supported? FileUtils.makedirs mib_dir - mib_hash = `smidump -f python #{module_file}` + mib_hash, error, status = capture_command('smidump', '-f', 'python', module_file.to_s) + unless status.success? + warn "*** Import failed for: #{module_file}: #{error.strip} ***" + return nil + end mib = eval_mib_data(mib_hash) if mib module_name = mib["moduleName"] raise "#{module_file}: invalid file format; no module name" unless module_name + unless module_name.is_a?(String) && MODULE_NAME_PATTERN.match?(module_name) + raise InvalidMIBError, "#{module_file}: invalid module name" + end if mib["nodes"] oid_hash = {} mib["nodes"].each { |key, value| oid_hash[key] = value["oid"] } @@ -118,14 +131,45 @@ def list_imported(regex=//, mib_dir=DEFAULT_MIB_PATH) private def eval_mib_data(mib_hash) - ruby_hash = mib_hash. - gsub(':', '=>'). # fix hash syntax - gsub('(', '[').gsub(')', ']'). # fix tuple syntax - sub('FILENAME =', 'filename ='). # get rid of constants - sub('MIB =', 'mib =') - mib = nil - eval(ruby_hash) - mib + raise InvalidMIBError, 'MIB converter output is too large' if mib_hash.bytesize > MAX_CONVERTER_OUTPUT_SIZE + + assignment = mib_hash.match(/(?:\A|\n)\s*MIB\s*=\s*/) + raise InvalidMIBError, 'MIB converter output does not contain a MIB assignment' unless assignment + + SNMP::PythonLiteralParser.new(mib_hash, offset: assignment.end(0)).parse + rescue SNMP::PythonLiteralParser::ParseError => e + raise InvalidMIBError, e.message + end + + def capture_command(*command) + output = String.new + error = String.new + status = nil + + Open3.popen3(*command) do |stdin, stdout, stderr, wait_thread| + stdin.close + streams = { stdout => [output, MAX_CONVERTER_OUTPUT_SIZE], stderr => [error, 65_536] } + + until streams.empty? + IO.select(streams.keys)&.first&.each do |stream| + begin + chunk = stream.read_nonblock(16_384) + buffer, limit = streams.fetch(stream) + if buffer.bytesize + chunk.bytesize > limit + Process.kill('KILL', wait_thread.pid) + raise InvalidMIBError, 'MIB converter output is too large' + end + buffer << chunk + rescue EOFError + streams.delete(stream) + stream.close + end + end + end + status = wait_thread.value + end + + [output, error, status] end end # class methods diff --git a/lib/snmp/python_literal_parser.rb b/lib/snmp/python_literal_parser.rb new file mode 100644 index 0000000000000..042b7aeda1fcf --- /dev/null +++ b/lib/snmp/python_literal_parser.rb @@ -0,0 +1,185 @@ +# frozen_string_literal: true + +module SNMP + # Parses the primitive Python literal subset emitted by `smidump -f python`. + class PythonLiteralParser + MAX_DEPTH = 64 + + class ParseError < StandardError; end + + # @param source [String] Python literal source + # @param offset [Integer] offset at which the literal starts + def initialize(source, offset: 0) + @source = source + @offset = offset + end + + # @return [Hash] parsed MIB dictionary + # @raise [ParseError] if source contains anything except primitive literals + def parse + value = parse_value(0) + skip_ignored + raise ParseError, 'Unexpected content after MIB data' unless eof? + raise ParseError, 'MIB data must be a dictionary' unless value.is_a?(Hash) + + value + end + + private + + def parse_value(depth) + raise ParseError, 'MIB data is nested too deeply' if depth > MAX_DEPTH + + skip_ignored + case current + when '{' then parse_hash(depth + 1) + when '[', '(' then parse_sequence(depth + 1) + when "'", '"' then parse_string + when '-', '0'..'9' then parse_integer + else parse_keyword + end + end + + def parse_hash(depth) + consume('{') + result = {} + skip_ignored + return consume('}') && result if current == '}' + + loop do + key = parse_value(depth) + raise ParseError, 'MIB dictionary keys must be strings' unless key.is_a?(String) + + skip_ignored + consume(':') + result[key] = parse_value(depth) + skip_ignored + break if current == '}' + + consume(',') + skip_ignored + break if current == '}' + end + consume('}') + result + end + + def parse_sequence(depth) + closer = current == '[' ? ']' : ')' + @offset += 1 + result = [] + skip_ignored + return consume(closer) && result if current == closer + + loop do + result << parse_value(depth) + skip_ignored + break if current == closer + + consume(',') + skip_ignored + break if current == closer + end + consume(closer) + result + end + + def parse_string + quote = current + consume(quote) + result = String.new + until eof? + char = take + return result if char == quote + + unless char == '\\' + result << char + next + end + + raise ParseError, 'Unterminated escape sequence' if eof? + + result << unescape(take) + end + raise ParseError, 'Unterminated string' + end + + def unescape(escaped) + return escaped if ['\\', "'", '"'].include?(escaped) + return [read_digits(2, 16, '\\x')].pack('U') if escaped == 'x' + return [read_digits(4, 16, '\\u')].pack('U') if escaped == 'u' + return [read_octal(escaped)].pack('U') if escaped.match?(/[0-7]/) + + replacements = { 'n' => "\n", 'r' => "\r", 't' => "\t", 'b' => "\b", 'f' => "\f", 'v' => "\v", 'a' => "\a" } + replacements.fetch(escaped) { raise ParseError, "Unsupported escape sequence \\#{escaped}" } + end + + def read_digits(length, base, prefix) + digits = @source[@offset, length] + unless digits&.length == length && digits.match?(base == 16 ? /\A[0-9A-Fa-f]+\z/ : /\A[0-7]+\z/) + raise ParseError, "Invalid #{prefix} escape sequence" + end + + @offset += length + digits.to_i(base) + end + + def read_octal(first_digit) + digits = first_digit + 2.times do + break unless current&.match?(/[0-7]/) + + digits << take + end + digits.to_i(8) + end + + def parse_integer + token = @source[@offset..].match(/\A-?\d+/)&.[](0) + raise ParseError, 'Invalid integer' unless token + + @offset += token.length + Integer(token, 10) + end + + def parse_keyword + token = @source[@offset..].match(/\A[A-Za-z_][A-Za-z0-9_]*/)&.[](0) + @offset += token.length if token + return nil if token == 'None' + return true if token == 'True' + return false if token == 'False' + + raise ParseError, "Unsupported MIB expression #{token.inspect}" + end + + def skip_ignored + loop do + @offset += 1 while !eof? && @source[@offset].match?(/\s/) + break unless current == '#' + + @offset += 1 until eof? || current == "\n" + end + end + + def consume(expected) + raise ParseError, "Expected #{expected.inspect}" unless current == expected + + @offset += 1 + true + end + + def take + char = current + @offset += 1 + char + end + + def current + @source[@offset] + end + + def eof? + @offset >= @source.length + end + end +end diff --git a/spec/lib/msf/core/rpc/v10/service_spec.rb b/spec/lib/msf/core/rpc/v10/service_spec.rb new file mode 100644 index 0000000000000..ef15e3c913ebf --- /dev/null +++ b/spec/lib/msf/core/rpc/v10/service_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require 'msf/core/rpc/v10/service' + +RSpec.describe Msf::RPC::Service do + let(:request_class) do + Struct.new(:request_method, :headers, :body) do + def method + request_method + end + end + end + + subject(:service) do + described_class.allocate.tap do |instance| + instance.handlers = { 'health' => health_handler } + instance.str_encoding = Encoding::UTF_8 + instance.dispatcher_timeout = 1 + end + end + + let(:health_handler) do + Class.new do + def rpc_check_noauth + { 'status' => 'UP' } + end + end.new + end + + it 'rejects an oversized body before MessagePack deserialization' do + request = request_class.new('POST', { 'Content-Type' => 'binary/message-pack' }, 'A' * (described_class::MAX_REQUEST_SIZE + 1)) + allow(MessagePack).to receive(:unpack).and_call_original + + expect { service.process(request) }.to raise_error(ArgumentError, /request body is too large/i) + expect(MessagePack).not_to have_received(:unpack) + end + + it 'continues to process a valid request within the limit' do + body = MessagePack.pack(['health.check']) + request = request_class.new('POST', { 'Content-Type' => 'binary/message-pack' }, body) + + expect(service.process(request)).to eq('status' => 'UP') + end +end diff --git a/spec/lib/rex/proto/http/packet_spec.rb b/spec/lib/rex/proto/http/packet_spec.rb index f052f753b9709..9f120465d2432 100644 --- a/spec/lib/rex/proto/http/packet_spec.rb +++ b/spec/lib/rex/proto/http/packet_spec.rb @@ -1,7 +1,29 @@ require 'spec_helper' +require 'rex/proto/http/packet' +require 'rex/proto/http/packet/header' +require 'rex/proto/http/request' +require 'support/shared/examples/hash_with_insensitive_access' RSpec.describe Rex::Proto::Http::Packet do + describe '#parse with max_body_size' do + subject(:request) { Rex::Proto::Http::Request.new } + + it 'rejects a declared body larger than the configured maximum before buffering it' do + result = request.parse("POST / HTTP/1.1\r\nContent-Length: 5\r\n\r\n", max_body_size: 4) + + expect(result).to eq(Rex::Proto::Http::Packet::ParseCode::Error) + expect(request.error).to be_a(ArgumentError) + end + + it 'rejects chunked bodies when their decoded size exceeds the configured maximum' do + result = request.parse("POST / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n", max_body_size: 4) + + expect(result).to eq(Rex::Proto::Http::Packet::ParseCode::Error) + expect(request.error).to be_a(ArgumentError) + end + end + it_behaves_like "hash with insensitive keys" describe "#parse" do diff --git a/spec/lib/snmp/mib_spec.rb b/spec/lib/snmp/mib_spec.rb new file mode 100644 index 0000000000000..4af1b3d26bdcc --- /dev/null +++ b/spec/lib/snmp/mib_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require 'snmp/mib' +require 'rbconfig' + +RSpec.describe SNMP::MIB do + describe '.import_module' do + let(:mib_output) do + <<~PYTHON + FILENAME = "safe.mib" + MIB = {"moduleName": "SAFE-MIB", "nodes": {"safeNode": {"oid": (1, 3, 6, 1)}}} + PYTHON + end + + it 'passes a caller-controlled filename to smidump as a separate argument' do + status = instance_double(Process::Status, success?: true) + allow(described_class).to receive(:import_supported?).and_return(true) + expect(described_class).to receive(:capture_command).with('smidump', '-f', 'python', 'name; touch injected').and_return([mib_output, '', status]) + + Dir.mktmpdir do |directory| + expect(described_class.import_module('name; touch injected', directory)).to eq('SAFE-MIB') + end + end + + it 'rejects executable expressions in smidump output without evaluating them' do + status = instance_double(Process::Status, success?: true) + allow(described_class).to receive(:import_supported?).and_return(true) + allow(described_class).to receive(:capture_command).and_return(['MIB = Kernel.system("touch injected")', '', status]) + + Dir.mktmpdir do |directory| + expect { described_class.import_module('unsafe.mib', directory) }.to raise_error(SNMP::MIB::InvalidMIBError) + end + end + + it 'rejects a module name that would escape the output directory' do + status = instance_double(Process::Status, success?: true) + allow(described_class).to receive(:import_supported?).and_return(true) + allow(described_class).to receive(:capture_command).and_return([mib_output.sub('SAFE-MIB', '../escape'), '', status]) + + Dir.mktmpdir do |directory| + expect { described_class.import_module('unsafe.mib', directory) }.to raise_error(SNMP::MIB::InvalidMIBError) + end + end + + it 'bounds converter output while it is being read' do + stub_const('SNMP::MIB::MAX_CONVERTER_OUTPUT_SIZE', 65_536) + command = [RbConfig.ruby, '-e', "STDOUT.write('A' * #{SNMP::MIB::MAX_CONVERTER_OUTPUT_SIZE + 1})"] + + expect { described_class.send(:capture_command, *command) }.to raise_error(SNMP::MIB::InvalidMIBError, /too large/) + end + end + + describe 'Python literal compatibility' do + it 'decodes hexadecimal, Unicode, and octal string escapes' do + mib = described_class.send(:eval_mib_data, 'MIB = {"moduleName":"SAFE-MIB","description":"\\x41\\u00e9\\101","nodes":{}}') + + expect(mib['description']).to eq("A\u00e9A") + end + end +end diff --git a/spec/support/shared/examples/msf/db_manager/import/metasploit_framework/zip.rb b/spec/support/shared/examples/msf/db_manager/import/metasploit_framework/zip.rb index 17269d46e5138..7715a7e20f105 100644 --- a/spec/support/shared/examples/msf/db_manager/import/metasploit_framework/zip.rb +++ b/spec/support/shared/examples/msf/db_manager/import/metasploit_framework/zip.rb @@ -95,5 +95,43 @@ def find_extracted_dir expect(File.exist?(File.join(controlled_tmpdir, 'escaped', 'pwned.txt'))).to be false end end + + context 'with a zip entry resolving to a sibling that shares the extraction prefix' do + let(:zip_path) { File.join(controlled_tmpdir, 'malicious.zip') } + + before do + create_msf_zip(zip_path, { + '../malicious_evil/pwned.txt' => 'pwned', + 'legit.xml' => valid_msf_xml + }) + end + + it 'does not extract the sibling-prefix entry' do + begin + framework.db.import_file(filename: zip_path) + rescue Msf::DBImportError + # Expected - the minimal XML is detected but may not fully import. + end + + extracted_dir = find_extracted_dir + sibling_dir = File.join(File.dirname(extracted_dir), 'malicious_evil') + expect(File.exist?(File.join(sibling_dir, 'pwned.txt'))).to be false + end + end + + describe '#is_child_of?' do + it 'accepts the extraction root and descendants' do + root = File.expand_path(File.join(controlled_tmpdir, 'archive')) + + expect(framework.db.send(:is_child_of?, root, root)).to be true + expect(framework.db.send(:is_child_of?, root, File.join(root, 'loot', 'file.bin'))).to be true + end + + it 'rejects sibling paths that merely share the extraction root prefix' do + root = File.expand_path(File.join(controlled_tmpdir, 'archive')) + + expect(framework.db.send(:is_child_of?, root, "#{root}_evil/file.bin")).to be false + end + end end end