Merge pull request #21677 from bwatters-r7/fix/http_relay_uripath

Fix bug where relay server failed to work if the attacker used the UR…

Author: jheysel-r7

lib/msf/core/exploit/remote/http_server/relay.rb

@@ -21,22 +21,29 @@ def initialize(info = {})
             OptInt.new('RELAY_TIMEOUT', [true, 'Seconds that the relay socket will wait for a response after the client has initiated communication.', 25])
           ], self.class
         )
+        deregister_options('URIPATH')
         @relay_clients = {}
         @relay_clients_mutex = Mutex.new
       end
 
+      # Every request, regardless of path, is serviced by the relay state machine - clients
+      # get bounced between targets via 307 redirects to randomly generated paths, so there's
+      # no single meaningful URIPATH to relay traffic through. Route the base HttpServer's
+      # resource registration straight to #on_relay_request at '/' instead of letting it mount
+      # its own (otherwise unused) resource_uri resource: Rex::Proto::Http::Server dispatches
+      # requests to the longest matching path prefix, so a second, more specific resource would
+      # silently swallow any request that happened to match it.
       def start_service(opts = {})
         @logger = opts['Logger'] || self
 
+        opts['Uri'] = {
+          'Proc' => Proc.new { |cli, req| on_relay_request(cli, req) },
+          'Path' => '/'
+        }
+
         super
 
         @http_relay_service = self.service
-
-        relay_path = '/'
-        add_resource(
-          'Proc' => Proc.new { |cli, req| on_relay_request(cli, req) },
-          'Path' => relay_path
-        )
       end
 
       def on_relay_request(cli, req)

spec/lib/msf/core/exploit/remote/relay/http_spec.rb

@@ -145,6 +145,33 @@ def get_client_state(server)
     end
   end
 
+  describe '#start_service' do
+    let(:service_double) do
+      service = double('service')
+      allow(service).to receive(:server_name=)
+      allow(service).to receive(:add_resource)
+      service
+    end
+
+    before do
+      allow(Rex::ServiceManager).to receive(:start).and_return(service_double)
+    end
+
+    it 'mounts a single catch-all resource at "/" regardless of URIPATH' do
+      relay_server.datastore['URIPATH'] = '/custom-uripath'
+
+      mounted_paths = []
+      allow(service_double).to receive(:add_resource) { |path, _opts| mounted_paths << path }
+
+      relay_server.start_service
+
+      # A second resource mounted at the (longer, more specific) URIPATH would silently swallow
+      # any request matching it, since Rex::Proto::Http::Server routes to the longest matching
+      # path prefix and the base HttpServer#on_request_uri handler for that resource is a no-op.
+      expect(mounted_paths).to eq(['/'])
+    end
+  end
+
   describe 'Target Iteration and Exhaustion' do
     let(:req1) { create_request("NTLM #{type1_b64}") }
     let(:req3) { create_request("NTLM #{type3_b64}") }