Update logic for aux modules having called report_vuln already

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)

lib/msf/base/simple/auxiliary.rb

@@ -176,7 +176,7 @@ def self.job_run_proc(ctx, &block)
       begin
         job_listener.start run_uuid
         mod.check_code = nil if mod.respond_to?(:check_code=)
-        mod.vuln_attempt_recorded = false if mod.respond_to?(:vuln_attempt_recorded=)
+        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)

lib/msf/core/auxiliary.rb

@@ -187,10 +187,11 @@ def fail_with(reason, msg = nil)
   attr_accessor  :check_code
 
   #
-  # Whether a vuln attempt was already recorded during this run (used to
-  # prevent duplicate attempts when report_failure is called later).
+  # 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  :vuln_attempt_recorded
+  attr_accessor  :last_vuln_attempt
 
   attr_accessor :queue
 

lib/msf/core/auxiliary/multiple_target_hosts.rb

@@ -22,12 +22,12 @@ def check
     nmod = replicant
     result = nmod.check_host(datastore['RHOST'])
 
-    # Propagate the vuln_attempt_recorded flag 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 skip creating a duplicate.
-    if nmod.respond_to?(:vuln_attempt_recorded) && nmod.vuln_attempt_recorded && respond_to?(:vuln_attempt_recorded=)
-      self.vuln_attempt_recorded = true
+    # 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

lib/msf/core/auxiliary/report.rb

@@ -326,17 +326,19 @@ def report_vuln(opts={})
     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)
 
-    # Signal that a vuln attempt was already recorded for this run so that
-    # report_failure (called later by the job wrapper) can skip creating a
-    # duplicate attempt in do_report_failure_or_success.
-    self.vuln_attempt_recorded = true if self.respond_to?(:vuln_attempt_recorded=)
+    # 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

@@ -55,7 +55,7 @@ def peer
 # The command handler when launched from the console
 #
 def run
-  @scanner_run_completed = true
+  @scanner_run_completed = false
   @show_progress = datastore['ShowProgress']
   @show_percent  = datastore['ShowProgressPercent'].to_i
 
@@ -141,7 +141,6 @@ def run
             print_status("Error: #{targ}: #{e.class} #{e.message}")
             elog("Error running against host #{targ}", error: e)
           ensure
-            nmod.report_failure
             nmod.cleanup
           end
         end
@@ -230,7 +229,6 @@ def run
             rescue ::Exception => e
               print_status("Error: #{mybatch[0]}-#{mybatch[-1]}: #{e}")
             ensure
-              nmod.report_failure
               nmod.cleanup
             end
           end
@@ -276,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,22 +79,23 @@ def report_exploit_failure(opts)
 
     vuln = nil
     if rids.present?
-      # Skip vuln lookup entirely when the check result indicates the target
-      # is not vulnerable.  A Safe/Unknown check on port 80 should not
-      # attach a VulnAttempt to an unrelated vuln on port 9200 just because
-      # the module refs happen to match.
-      safe_fail_reasons = [Msf::Module::Failure::NotVulnerable, Msf::Module::Failure::Unknown]
-      unless safe_fail_reasons.include?(opts[:fail_reason])
+      # 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.  Vulns created by `check` (via module_command_dispatcher's
-        # report_vuln) often have no associated service, so a service-scoped
-        # query will miss them.
-        if vuln.nil? && svc
-          vuln = find_vuln_by_refs(rids, host, nil, false)
+        # 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
@@ -173,14 +174,19 @@ def do_report_failure_or_success(opts)
         # Create a references map from the module list
         ref_objs = ::Mdm::Ref.where(name: ref_names)
 
-        # Skip vuln lookup when the result indicates the target is not
-        # vulnerable — avoids attaching a VulnAttempt to an unrelated vuln
-        # on a different port that happens to share the same refs.
-        safe_fail_reasons = [Msf::Module::Failure::NotVulnerable, Msf::Module::Failure::Unknown]
-        unless safe_fail_reasons.include?(freason)
+        # 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)
-          vuln ||= find_vuln_by_refs(ref_objs, host, nil, false) if svc && vuln.nil?
+          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
 
@@ -201,7 +207,7 @@ def do_report_failure_or_success(opts)
       # 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 vuln_attempt_recorded flag on the module).
+      # 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)

lib/msf/core/exploit.rb

@@ -1494,10 +1494,11 @@ def handle_exception e
   attr_accessor  :fail_detail
 
   #
-  # Whether a vuln attempt was already recorded during this run (used to
-  # prevent duplicate attempts when report_failure is called later).
+  # 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  :vuln_attempt_recorded
+  attr_accessor  :last_vuln_attempt
 
   #
   # The list of targets.

lib/msf/core/module/failure.rb

@@ -93,6 +93,8 @@ def report_failure
       info[:port] = self.datastore['RPORT']
       if self.class.ancestors.include?(Msf::Exploit::Remote::Tcp)
         info[:proto] = 'tcp'
+      elsif self.class.ancestors.include?(Msf::Exploit::Remote::Udp)
+        info[:proto] = 'udp'
       end
     end
 
@@ -127,9 +129,9 @@ def report_failure
     # check_code is available, update the existing attempt so it carries the
     # check result details (the attempt created by report_vuln may not have
     # had the check_code yet because it runs before job_run_proc stores it).
-    if self.respond_to?(:vuln_attempt_recorded) && self.vuln_attempt_recorded
+    if self.respond_to?(:last_vuln_attempt) && self.last_vuln_attempt
       if self.respond_to?(:check_code) && self.check_code.is_a?(Msf::Exploit::CheckCode)
-        _enrich_existing_vuln_attempt(info)
+        _enrich_existing_vuln_attempt(info, self.last_vuln_attempt)
       end
       info[:skip_vuln_attempt] = true
     end
@@ -139,28 +141,51 @@ def report_failure
 
   private
 
-  # Update the most recent VulnAttempt for this module/host with check code
-  # details that were not available when report_vuln originally created it.
-  def _enrich_existing_vuln_attempt(info)
+  # Update the VulnAttempt for this module/host with check code details that
+  # were not available when report_vuln originally created it.
+  #
+  # @param info [Hash] enrichment data built by report_failure
+  # @param recorded_attempt [Mdm::VulnAttempt, true] the attempt object stored
+  #   by report_vuln, or +true+ if only the flag was propagated (legacy/fallback).
+  def _enrich_existing_vuln_attempt(info, recorded_attempt = nil)
     return unless framework.db&.active
 
-    host = info[:host]
-    return unless host
-
-    host_obj = if host.is_a?(::Mdm::Host)
-                 host
-               else
-                 wspace = info[:workspace] || framework.db.find_workspace(workspace)
-                 framework.db.get_host(workspace: wspace, address: host.to_s)
-               end
-    return unless host_obj
-
-    # Find the most recent vuln attempt for this module on this host
-    attempt = ::Mdm::VulnAttempt
+    # Use the stored attempt directly when available — avoids a racy
+    # re-query that could match the wrong row under concurrency.
+    attempt = recorded_attempt if recorded_attempt.is_a?(::Mdm::VulnAttempt)
+
+    # Fallback: re-query if we only have the boolean flag (e.g. propagated
+    # through a replicant that only forwarded +true+).
+    if attempt.nil?
+      host = info[:host]
+      return unless host
+
+      host_obj = if host.is_a?(::Mdm::Host)
+                   host
+                 else
+                   wspace = info[:workspace] || framework.db.find_workspace(workspace)
+                   framework.db.get_host(workspace: wspace, address: host.to_s)
+                 end
+      return unless host_obj
+
+      scope = ::Mdm::VulnAttempt
                 .joins(:vuln)
                 .where(module: fullname, vulns: { host_id: host_obj.id })
-                .order(attempted_at: :desc)
-                .first
+
+      # Narrow by service attributes when available so we don't match an
+      # attempt against a different service on the same host (e.g. port 80
+      # vs 9200, or TCP vs UDP on the same port).
+      if info[:port]
+        service_conditions = { port: info[:port] }
+        service_conditions[:proto] = info[:proto].to_s.downcase if info[:proto]
+
+        scope = scope.joins(vuln: :service)
+                     .where(services: service_conditions)
+      end
+
+      attempt = scope.order(attempted_at: :desc).first
+    end
+
     return unless attempt
 
     updates = {}

lib/msf/ui/console/module_command_dispatcher.rb

@@ -178,9 +178,17 @@ def report_vuln(instance, checkcode = nil)
     opts = {
       workspace: instance.workspace,
       host: instance.respond_to?(:target_host) && instance.target_host ? instance.target_host : instance.datastore['RHOST'],
-      proto: instance.datastore['PROTO'] || 'tcp',
+      proto: if instance.class.ancestors.include?(Msf::Exploit::Remote::Udp)
+               'udp'
+             else
+               'tcp'
+             end,
       name: instance.name,
-      info: "This was flagged as vulnerable by the explicit check of #{instance.fullname}.",
+      info: if checkcode == Msf::Exploit::CheckCode::Appears
+              "Target appears vulnerable based on the explicit check of #{instance.fullname}."
+            else
+              "Vulnerability confirmed by the explicit check of #{instance.fullname}."
+            end,
       refs: instance.references
     }