fix(console): prevent RHOSTS temp file deletion after services -R

[BenoitDePaoli]

Fix: RHOSTS temp file deleted before module run when using services ... -R

Summary

Using services -p <ports> -u -R could set RHOSTS to a file:/tmp/msf-db-rhosts-* path that vanished immediately upon running a module. This resulted in modules starting with an empty or ineffective target list. The root cause was that the temporary file was created via Rex::Quickfile in set_rhosts_from_addrs and the only Ruby reference to the object went out of scope. Under recent Ruby / Rex behavior, the file was unlinked when the object was garbage collected (often triggered right as a module initialized and allocated additional objects).

Root Cause

set_rhosts_from_addrs wrote the host list to a Rex::Quickfile, closed it, and returned without retaining a reference. If Rex::Quickfile (or an underlying implementation analogous to Tempfile) registers a finalizer that unlinks the file, the OS file disappeared as soon as GC (garbage collector) finalized the object. GC frequently occurred when a module was launched due to new allocations, making the deletion appear to be caused by the module itself.

Fix Implemented

We now retain references to created Rex::Quickfile instances in an instance variable @persisted_rhosts_files, preventing garbage collection (and therefore unlink) for the lifetime of the console process. For small host lists (≤5 hosts), behavior is unchanged: the list is stored inline in the RHOSTS datastore.

Test Coverage

Note on Reproduction via db_import

The issue is most easily observed after importing a larger host/service set from an Nmap XML file using db_import <scan.xml>. Imported scans commonly contain more than five hosts, causing set_rhosts_from_addrs to choose the temp file path variant (file:/tmp/msf-db-rhosts-*). More you have larger host/service more you have chance to trigger the garbage collector at the run.

1. Import large number of data
   From namp scan:

db_import nmap_scan.xml

Or create many fake hosts/services ( > 60 ):
On bash:

echo "workspace -a Demo" > add_rdps.rc
for i in $(seq 1 60); do
  echo "hosts -a 192.168.0.$i"
  echo "services -a -p 3389 -r tcp -s rdp 192.168.0.$i"
done >> add_rdps.rc

msfconsole -r add_rdps.rc

2. Select and run a module:

use auxiliary/scanner/rdp/rdp_scanner
services -p 3389 -u -R
ls /tmp/msf-db-rhosts-XXXXXXXXX
run
ls /tmp/msf-db-rhosts-XXXXXXXXX

Note the displayed file path (e.g. file:/tmp/msf-db-rhosts-...).

The file has been removed and the module didn't scan the targets. Because the garbage collector clean the file.

Apply the fix and rerun the step 1 and 2 above you will see that the scan run properly and the file is not clean by the GC.

[smcintyre-r7]

Deleted my previous comment. I didn't realize at first that this actually seems to be fixing a race condition and not intended to store the temporary files more long term. The proposed solution seems kinda reasonable, but ideally, we'd clean the file up once the module had completed and not when the framework shutsdown.

[BenoitDePaoli]

The issue with deleting the file after the module completes is that if I want to rerun the module because it failed or for other reasons, I'll have to rerun the command "service ... -R" to recreate a temporary file.

[smcintyre-r7]

So we had a chance to look through the tempfile API a bit and what we noticed is that when the file is created in this way, it's automatically removed by the garbage collector. If however, it's created using the .create method, it will not automatically delete the file.

That means that instead of keeping a reference here, we should be able to achieve the same fix by adjusting L87 from rhosts_file = Rex::Quickfile.new("msf-db-rhosts-") to rhosts_file = Rex::Quickfile.create("msf-db-rhosts-"). GitHub won't let me suggest changes on lines you didn't write. With that in place, I'd then remove these lines since we don't need to explicitly keep a reference.

[BenoitDePaoli]

The issue with "create" is that the file is not removed once the Metasploit console is exited, so the temp file will stay on the filesystem. Is this something we want?