Land rapid7#2002, @jlee-r7's patch for better handling uri resources

Author: jvazquez-r7

lib/msf/core/exploit/http/server.rb

@@ -36,6 +36,9 @@ def initialize(info = {})
 			], Exploit::Remote::HttpServer
 		)
 
+		# Used to keep track of resources added to the service manager by
+		# this module. see #add_resource and #cleanup
+		@my_resources = []
 		@service_path = nil
 	end
 
@@ -202,6 +205,39 @@ def start_service(opts = {})
 		add_resource(uopts)
 	end
 
+	# Set {#on_request_uri} to handle the given +uri+ in addition to the one
+	# specified by the user in URIPATH.
+	#
+	# @note This MUST be called from {#primer} so that the service has been set
+	# up but we have not yet entered the listen/accept loop.
+	#
+	# @param uri [String] The resource URI that should be handled by
+	#   {#on_request_uri}.
+	# @return [void]
+	def hardcoded_uripath(uri)
+		proc = Proc.new do |cli, req|
+			on_request_uri(cli, req)
+		end
+
+		vprint_status("Adding hardcoded uri #{uri}")
+		begin
+			add_resource({'Path' => uri, 'Proc' => proc})
+		rescue RuntimeError => e
+			print_error("This module requires a hardcoded uri at #{uri}. Can't run while other modules are using it.")
+			raise e
+		end
+	end
+
+	# Take care of removing any resources that we created
+	def cleanup
+		# Must dup here because remove_resource modifies @my_resources
+		@my_resources.dup.each do |resource|
+			remove_resource(resource)
+		end
+
+		super
+	end
+
 	#
 	# Return a Hash containing a best guess at the actual browser and operating
 	# system versions, based on the User-Agent header.
@@ -358,9 +394,16 @@ def report_user_agent(address, request, client_opts={})
 	# NOTE: Calling #add_resource will change the results of subsequent calls
 	# to #get_resource!
 	#
+	# @return (see Rex::Service#add_resource)
 	def add_resource(opts)
 		@service_path = opts['Path']
-		service.add_resource(opts['Path'], opts)
+		res = service.add_resource(opts['Path'], opts)
+
+		# This has to go *after* the call to service.add_resource in case
+		# the service manager doesn't like it for some reason and raises.
+		@my_resources.push(opts['Path'])
+
+		res
 	end
 
 	#
@@ -455,7 +498,11 @@ def srvhost_addr
 	# Removes a URI resource.
 	#
 	def remove_resource(name)
-		service.remove_resource(name)
+		# Guard against removing resources added by other modules
+		if @my_resources.include?(name)
+			@my_resources.delete(name)
+			service.remove_resource(name)
+		end
 	end
 
 	#

modules/exploits/windows/browser/adobe_flash_mp4_cprt.rb

@@ -170,18 +170,14 @@ def get_target(agent)
 		end
 	end
 
+	def primer
+		# "/test.mp4" is currently hard-coded in the swf file, so we need to add to resource
+		hardcoded_uripath("/test.mp4")
+	end
+
 	def exploit
 		@swf = create_swf
 		super
-
-		#
-		# "/test.mp4" is currently hard-coded in the swf file, so we need to add to resource
-		#
-		proc = Proc.new do |cli, req|
-			self.add_resource({'Path' => "/test.mp4", 'Proc' => proc}) rescue nil
-			on_request_uri(cli, req)
-		end
-
 	end
 
 	def on_request_uri(cli, request)
@@ -275,12 +271,6 @@ def on_request_uri(cli, request)
 		send_response(cli, html, {'Content-Type'=>'text/html'})
 	end
 
-	def cleanup
-		print_status("Removing mp4 resource")
-		remove_resource('/test.mp4') rescue nil
-		super
-	end
-
 	def create_swf
 		path = ::File.join( Msf::Config.install_root, "data", "exploits", "CVE-2012-0754.swf" )
 		fd = ::File.open( path, "rb" )

modules/exploits/windows/browser/adobe_flash_otf_font.rb

@@ -204,16 +204,15 @@ def on_request_uri(cli, request)
 
 		html = html.gsub(/^\t\t/, '')
 
-		# we need to handle direct /pay.txt requests
-		proc = Proc.new do |cli, req|
-			on_request_uri(cli, req)
-		end
-		add_resource({'Path' => "/#{@resource_name}.txt", 'Proc' => proc}) rescue nil
-
 		print_status("Sending HTML")
 		send_response(cli, html, {'Content-Type'=>'text/html'})
 	end
 
+	def primer
+		# we need to handle direct /pay.txt requests
+		hardcoded_uripath("/#{@resource_name}.txt")
+	end
+
 	def exploit
 		@swf = create_swf
 		@resource_name = Rex::Text.rand_text_alpha(5)
@@ -235,10 +234,4 @@ def create_swf
 		return swf
 	end
 
-	def cleanup
-		vprint_status("Removing txt resource")
-		remove_resource("/#{@resource_name}.txt") rescue nil
-		super
-	end
-
 end

modules/exploits/windows/browser/citrix_gateway_actx.rb

@@ -60,6 +60,10 @@ def initialize(info = {})
 			'DefaultTarget'  => 0))
 	end
 
+	def primer
+		hardcoded_uripath("/epaq")
+	end
+
 	def exploit
 		@ocx = ::File.read(::File.join(Msf::Config.install_root, 'data', 'exploits', 'CVE-2011-2882', 'nsepa.ocx'))
 		super
@@ -184,12 +188,6 @@ def on_request_uri(cli, request)
 
 		html = html.gsub(/^\t\t/, '')
 
-		# we need to handle direct /epaq requests
-		proc = Proc.new do |cli, req|
-			on_request_uri(cli, req)
-		end
-
-		add_resource({'Path' => "/epaq", 'Proc' => proc}) rescue nil
 		print_status("Sending #{self.name} HTML")
 		send_response(cli, html, { 'Content-Type' => 'text/html' })
 	end

modules/exploits/windows/browser/honeywell_hscremotedeploy_exec.rb

@@ -55,6 +55,10 @@ def initialize(info={})
 			'DefaultTarget'  => 0))
 	end
 
+	def primer
+		hardcoded_uripath("/SystemDisplays/RemoteInstallWelcome.hta")
+	end
+
 	def exploit
 		@var_exename = rand_text_alpha(5 + rand(5)) + ".exe"
 		@dropped_files = [
@@ -198,13 +202,6 @@ def on_request_uri(cli, request)
 		</html>
 		EOS
 
-		# we need to handle direct /SystemDisplays/RemoteInstallWelcome.hta requests
-		proc = Proc.new do |cli, req|
-			on_request_uri(cli, req)
-		end
-
-		add_resource({'Path' => "/SystemDisplays/RemoteInstallWelcome.hta", 'Proc' => proc}) rescue nil
-
 		print_status("Sending html")
 		send_response(cli, html, {'Content-Type'=>'text/html'})
 

spec/lib/msf/core/exploit/http/server_spec.rb

@@ -0,0 +1,89 @@
+
+# -*- coding:binary -*-
+require 'spec_helper'
+
+require 'msf/core'
+require 'msf/core/exploit/http/server'
+
+describe Msf::Exploit::Remote::HttpServer do
+	subject(:server_module) do
+		mod = Msf::Exploit.allocate
+		mod.extend described_class
+		mod.send(:initialize, {})
+
+		mod
+	end
+
+	let(:mock_service) do
+		mock_service = mock("service")
+		mock_service.stub(:server_name=)
+		mock_service.stub(:add_resource)
+
+		mock_service
+	end
+
+	before do
+		Rex::ServiceManager.stub(:start => mock_service)
+	end
+
+	describe "#add_resource" do
+		it "should call the ServiceManager's add_resource" do
+			server_module.start_service
+
+			mock_service.should_receive(:add_resource)
+			server_module.add_resource('Path' => 'foo')
+		end
+
+		it "should re-raise if the resource has already been added" do
+			server_module.start_service
+
+			mock_service.should_receive(:add_resource).ordered
+			mock_service.should_receive(:add_resource).ordered.and_raise(RuntimeError)
+
+			server_module.add_resource('Path' => 'foo')
+
+			expect { server_module.add_resource('Path' => 'foo') }.to raise_error
+		end
+
+	end
+
+	describe "#cleanup" do
+		it "should not remove resources if none were successfully added" do
+			server_module.should_not_receive(:remove_resource)
+			server_module.cleanup
+		end
+
+		it "should remove successfully-added resources" do
+			# setup
+			server_module.start_service
+			resources = [ 'a', 'b', 'c' ]
+			resources.each { |r| server_module.add_resource('Path' => r) }
+
+			# The service will add one resource as part of #start_service, so
+			# add that to the number that we added manually
+			server_module.should_receive(:remove_resource).exactly(resources.count + 1).times
+			server_module.cleanup
+		end
+
+	end
+
+	describe "#hardcoded_uripath" do
+		it "should call the ServiceManager's add_resource" do
+			server_module.start_service
+
+			mock_service.should_receive(:add_resource)
+			server_module.hardcoded_uripath('foo')
+		end
+
+		it "should re-raise if the resource has already been added" do
+			server_module.start_service
+
+			mock_service.should_receive(:add_resource).ordered.and_raise(RuntimeError)
+
+			expect { server_module.hardcoded_uripath('foo') }.to raise_error
+		end
+	end
+
+end
+
+