Merge pull request rapid7#21307 from adfoster-r7/improve-vuln-and-vuln-attempt-tracking

Improve vuln and vuln attempt tracking

Author: adfoster-r7

Gemfile.lock

@@ -353,17 +353,17 @@ GEM
       mutex_m
       railties (~> 7.0)
     metasploit-payloads (2.0.245)
-    metasploit_data_models (6.0.15)
-      activerecord (~> 7.0)
-      activesupport (~> 7.0)
+    metasploit_data_models (6.0.18)
+      activerecord (>= 7.0, < 8.1)
+      activesupport (>= 7.0, < 8.1)
       arel-helpers
       bigdecimal
       drb
       metasploit-concern
-      metasploit-model (~> 5.0.4)
+      metasploit-model (>= 5.0.4)
       mutex_m
       pg
-      railties (~> 7.0)
+      railties (>= 7.0, < 8.1)
       recog
       webrick
     metasploit_payloads-mettle (1.0.46)

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.2].define(version: 2026_01_30_124052) do
+ActiveRecord::Schema[7.2].define(version: 2026_04_11_000000) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -665,6 +665,8 @@
     t.integer "session_id"
     t.integer "loot_id"
     t.text "fail_detail"
+    t.string "check_code"
+    t.text "check_detail"
   end
 
   create_table "vuln_details", id: :serial, force: :cascade do |t|

lib/msf/base/simple/auxiliary.rb

@@ -175,9 +175,13 @@ def self.job_run_proc(ctx, &block)
     begin
       begin
         job_listener.start run_uuid
+        mod.check_code = nil if mod.respond_to?(:check_code=)
+        mod.last_vuln_attempt = nil if mod.respond_to?(:last_vuln_attempt=)
         mod.setup
         mod.framework.events.on_module_run(mod)
         result = block.call(mod)
+        # Store the check result if the block returned a CheckCode
+        mod.check_code = result if result.is_a?(Msf::Exploit::CheckCode)
         job_listener.completed(run_uuid, result, mod)
       rescue ::Exception => e
         job_listener.failed(run_uuid, e, mod)

lib/msf/core/auxiliary.rb

@@ -181,6 +181,18 @@ def fail_with(reason, msg = nil)
   #
   attr_accessor  :fail_detail
 
+  #
+  # The result of the last check invocation (a Msf::Exploit::CheckCode), if any
+  #
+  attr_accessor  :check_code
+
+  #
+  # The VulnAttempt object created during this run, or nil/false if none
+  # was recorded.  Used to prevent duplicate attempts when report_failure
+  # is called later and to enrich the attempt with check code details.
+  #
+  attr_accessor  :last_vuln_attempt
+
   attr_accessor :queue
 
 protected

lib/msf/core/auxiliary/multiple_target_hosts.rb

@@ -20,7 +20,17 @@ def check
     return Exploit::CheckCode::Unsupported unless has_check?
 
     nmod = replicant
-    nmod.check_host(datastore['RHOST'])
+    result = nmod.check_host(datastore['RHOST'])
+
+    # Propagate the last_vuln_attempt (which may be the actual VulnAttempt
+    # object) back from the replicant so that the ensure block in
+    # job_run_proc (which calls report_failure on the *original* instance)
+    # knows a vuln attempt was already created and can enrich it directly.
+    if nmod.respond_to?(:last_vuln_attempt) && nmod.last_vuln_attempt && respond_to?(:last_vuln_attempt=)
+      self.last_vuln_attempt = nmod.last_vuln_attempt
+    end
+
+    result
   end
 
 end

lib/msf/core/auxiliary/report.rb

@@ -314,11 +314,31 @@ def report_vuln(opts={})
         :fail_detail  => 'vulnerability identified',
         :fail_reason  => 'Untried', # Mdm::VulnAttempt::Status::UNTRIED, avoiding direct dependency on Mdm, used elsewhere in this module
         :module       => mname,
-        :username     => username  || "unknown"
+        :username     => username || self.owner || "unknown"
     }
 
+    # Enrich attempt with check code details when available.
+    # Accept an explicit check_code in opts (useful when the module knows the
+    # result before the framework sets self.check_code), falling back to the
+    # module-level accessor.
+    check_code = opts[:check_code]
+    check_code = self.check_code if check_code.nil? && self.respond_to?(:check_code)
+    if check_code.is_a?(Msf::Exploit::CheckCode)
+      attempt_info[:check_code] = check_code.code
+      attempt_info[:check_detail] = check_code.reason || check_code.message
+      attempt_info[:fail_detail] = nil
+      mapped_reason = Msf::Module::Failure.fail_reason_from_check_code(check_code)
+      attempt_info[:fail_reason] = mapped_reason if mapped_reason
+    end
+
     # TODO: figure out what opts are required and why the above logic doesn't match that of the db_manager method
-    framework.db.report_vuln_attempt(vuln, attempt_info)
+    attempt = framework.db.report_vuln_attempt(vuln, attempt_info)
+
+    # Store the attempt object so that report_failure (called later by the
+    # job wrapper) can enrich it directly without re-querying the DB.
+    if self.respond_to?(:last_vuln_attempt=)
+      self.last_vuln_attempt = attempt || true
+    end
 
     vuln
   end

lib/msf/core/auxiliary/scanner.rb

@@ -15,6 +15,19 @@ module Auxiliary::Scanner
 class AttemptFailed < Msf::Auxiliary::Failed
 end
 
+# Scanner modules handle per-host failure reporting through replicants
+# inside their run_host/run_batch threads.  Override the default
+# report_failure so that the parent-level call from job_run_proc's
+# ensure block does not create a duplicate or misattributed attempt
+# after a scan.  The check path (check_simple) still needs the
+# default report_failure behaviour, so we only skip when the scanner's
+# run method has executed.
+def report_failure
+  return if @scanner_run_completed
+
+  super
+end
+
 #
 # Initializes an instance of a recon auxiliary module
 #
@@ -42,6 +55,7 @@ def peer
 # The command handler when launched from the console
 #
 def run
+  @scanner_run_completed = false
   @show_progress = datastore['ShowProgress']
   @show_percent  = datastore['ShowProgressPercent'].to_i
 
@@ -260,6 +274,7 @@ def run
     print_status("Caught interrupt from the console...")
     return
   ensure
+    @scanner_run_completed = true
     seppuko!()
   end
 end

lib/msf/core/db_manager/exploit_attempt.rb

@@ -79,9 +79,25 @@ def report_exploit_failure(opts)
 
     vuln = nil
     if rids.present?
-      # Try to find an existing vulnerability with the same service & references
-      # or, if svc is nil, with the same host & references
-      vuln = find_vuln_by_refs(rids, host, svc, false)
+      # Only perform vuln lookup when no check_code is present (normal
+      # exploit flow) or the check result positively indicates vulnerability.
+      # Safe, Unknown, and Detected results should not associate this attempt
+      # with an existing vuln.  Only key off check_code — fail_reason alone
+      # is too broad (e.g. Failure::Unknown covers real exploit failures too).
+      vuln_check_codes = [Msf::Exploit::CheckCode::Appears.code, Msf::Exploit::CheckCode::Vulnerable.code]
+      if opts[:check_code].nil? || vuln_check_codes.include?(opts[:check_code])
+        # Try to find an existing vulnerability with the same service & references
+        # or, if svc is nil, with the same host & references
+        vuln = find_vuln_by_refs(rids, host, svc, false)
+
+        # Fall back to a host-only lookup when the service-scoped query found
+        # nothing.  Only match vulns with no associated service to avoid
+        # misattributing attempts to a vuln on a different service.
+        if svc && vuln.nil?
+          fallback_vuln = find_vuln_by_refs(rids, host, nil, false)
+          vuln = fallback_vuln if fallback_vuln && fallback_vuln.service_id.nil?
+        end
+      end
     end
 
     opts[:service] = svc
@@ -158,8 +174,20 @@ def do_report_failure_or_success(opts)
         # Create a references map from the module list
         ref_objs = ::Mdm::Ref.where(name: ref_names)
 
-        # Try find a matching vulnerability
-        vuln = find_vuln_by_refs(ref_objs, host, svc, false)
+        # Only perform vuln lookup when no check_code is present (normal
+        # exploit flow) or the check result positively indicates vulnerability.
+        # Safe, Unknown, and Detected results should not associate this attempt
+        # with an existing vuln.  Only key off check_code — fail_reason alone
+        # is too broad (e.g. Failure::Unknown covers real exploit failures too).
+        vuln_check_codes = [Msf::Exploit::CheckCode::Appears.code, Msf::Exploit::CheckCode::Vulnerable.code]
+        if opts[:check_code].nil? || vuln_check_codes.include?(opts[:check_code])
+          # Try find a matching vulnerability
+          vuln = find_vuln_by_refs(ref_objs, host, svc, false)
+          if svc && vuln.nil?
+            fallback_vuln = find_vuln_by_refs(ref_objs, host, nil, false)
+            vuln = fallback_vuln if fallback_vuln && fallback_vuln.service_id.nil?
+          end
+        end
       end
 
       attempt_info = {
@@ -170,12 +198,17 @@ def do_report_failure_or_success(opts)
         :module       => mname,
         :username     => username  || "unknown",
       }
+      attempt_info[:check_code] = opts[:check_code] if opts[:check_code]
+      attempt_info[:check_detail] = opts[:check_detail] if opts[:check_detail]
 
       attempt_info[:session_id] = opts[:session_id] if opts[:session_id]
       attempt_info[:loot_id]    = opts[:loot_id]    if opts[:loot_id]
 
-      # We have match, lets create a vuln_attempt record
-      if vuln
+      # We have match, lets create a vuln_attempt record.
+      # Skip if the caller already recorded a vuln attempt for this run
+      # (e.g. Auxiliary::Report#report_vuln sets skip_vuln_attempt via
+      # the last_vuln_attempt flag on the module).
+      if vuln && !opts[:skip_vuln_attempt]
         attempt_info[:vuln_id] = vuln.id
         vuln.vuln_attempts.create(attempt_info)
 
@@ -200,7 +233,8 @@ def do_report_failure_or_success(opts)
         attempt_info[:proto] = prot || Msf::DBManager::DEFAULT_SERVICE_PROTO
       end
 
-      host.exploit_attempts.create(attempt_info)
+      # check_code and check_detail are valid for VulnAttempt but not ExploitAttempt
+      host.exploit_attempts.create(attempt_info.except(:check_code, :check_detail))
     }
 
   end

lib/msf/core/exploit.rb

@@ -1493,6 +1493,13 @@ def handle_exception e
   #
   attr_accessor  :fail_detail
 
+  #
+  # The VulnAttempt object created during this run, or nil/false if none
+  # was recorded.  Used to prevent duplicate attempts when report_failure
+  # is called later and to enrich the attempt with check code details.
+  #
+  attr_accessor  :last_vuln_attempt
+
   #
   # The list of targets.
   #

lib/msf/core/exploit/remote/auto_check.rb

@@ -51,7 +51,8 @@ def with_prepended_auto_check
           name: fullname,
           username: respond_to?(:owner) ? owner : nil,
           refs: references,
-          info: description.strip
+          info: description.strip,
+          check_code: check_code
         }
 
         if respond_to?(:session) && session.respond_to?(:session_host)