Merge pull request #20777 from rapid7/revert-20424-enh/MS-9930/vuln_report

Revert "Vulnerability Report Enhancement"

Author: cgranleese-r7

Gemfile.lock

@@ -353,15 +353,12 @@ GEM
       mutex_m
       railties (~> 7.0)
     metasploit-payloads (2.0.237)
-    metasploit_data_models (6.0.11)
+    metasploit_data_models (6.0.9)
       activerecord (~> 7.0)
       activesupport (~> 7.0)
       arel-helpers
-      bigdecimal
-      drb
       metasploit-concern
       metasploit-model (>= 3.1)
-      mutex_m
       pg
       railties (~> 7.0)
       recog

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: 2025_07_21_114306) do
+ActiveRecord::Schema[7.2].define(version: 2025_02_04_172657) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -521,16 +521,6 @@
     t.string "netmask"
   end
 
-  create_table "service_links", force: :cascade do |t|
-    t.bigint "parent_id", null: false
-    t.bigint "child_id", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.index ["child_id"], name: "index_service_links_on_child_id"
-    t.index ["parent_id", "child_id"], name: "index_service_links_on_parent_id_and_child_id", unique: true
-    t.index ["parent_id"], name: "index_service_links_on_parent_id"
-  end
-
   create_table "services", id: :serial, force: :cascade do |t|
     t.integer "host_id"
     t.datetime "created_at", precision: nil
@@ -540,8 +530,7 @@
     t.string "name"
     t.datetime "updated_at", precision: nil
     t.text "info"
-    t.jsonb "resource", default: {}, null: false
-    t.index ["host_id", "port", "proto", "name", "resource"], name: "index_services_on_5_columns", unique: true
+    t.index ["host_id", "port", "proto"], name: "index_services_on_host_id_and_port_and_proto", unique: true
     t.index ["name"], name: "index_services_on_name"
     t.index ["port"], name: "index_services_on_port"
     t.index ["proto"], name: "index_services_on_proto"
@@ -697,7 +686,6 @@
     t.integer "vuln_attempt_count", default: 0
     t.integer "origin_id"
     t.string "origin_type"
-    t.jsonb "resource", default: {}, null: false
     t.index ["name"], name: "index_vulns_on_name"
     t.index ["origin_id"], name: "index_vulns_on_origin_id"
   end
@@ -815,7 +803,4 @@
     t.boolean "limit_to_network", default: false, null: false
     t.boolean "import_fingerprint", default: false
   end
-
-  add_foreign_key "service_links", "services", column: "child_id"
-  add_foreign_key "service_links", "services", column: "parent_id"
 end

lib/msf/core/db_manager/service.rb

@@ -6,14 +6,7 @@ def delete_service(opts)
   ::ApplicationRecord.connection_pool.with_connection {
     deleted = []
     opts[:ids].each do |service_id|
-      begin
-        service = Mdm::Service.find(service_id)
-      rescue ActiveRecord::RecordNotFound
-        # This happens when the service was the child of another service we have already deleted
-        # Deletion of children is automatic via dependent: :destroy on the association
-        dlog("Service with id #{service_id} already deleted")
-        next
-      end
+      service = Mdm::Service.find(service_id)
       begin
         deleted << service.destroy
       rescue
@@ -50,15 +43,11 @@ def find_or_create_service(opts)
   # +:workspace+:: the workspace for the service
   #
   # opts may contain
-  # +:name+::     the application layer protocol (e.g. ssh, mssql, smb)
-  # +:sname+::    an alias for the above
-  # +:info+::     detailed information about the service such as name and version information
-  # +:state+::    the current listening state of the service (one of: open, closed, filtered, unknown)
-  # +:resource+:: the resource this service is associated with, such as a a DN for an an LDAP object
-  #               base URI for a web application, pipe name for DCERPC service, etc.
-  # +:parents+::  a single service Hash or an Array of service Hash representing the parent services this
-  #               service is associated with, such as a HTTP service for a web application.
-  #`
+  # +:name+::  the application layer protocol (e.g. ssh, mssql, smb)
+  # +:sname+:: an alias for the above
+  # +:info+:: Detailed information about the service such as name and version information
+  # +:state+:: The current listening state of the service (one of: open, closed, filtered, unknown)
+  #
   # @return [Mdm::Service,nil]
   def report_service(opts)
     return if !active
@@ -80,7 +69,6 @@ def report_service(opts)
     if opts[:sname]
       opts[:name] = opts.delete(:sname)
     end
-    opts[:name] = opts[:name].to_s.downcase if opts[:name]
 
     if addr.kind_of? ::Mdm::Host
       host = addr
@@ -96,14 +84,7 @@ def report_service(opts)
 
     proto = opts[:proto] || Msf::DBManager::DEFAULT_SERVICE_PROTO
 
-    sopts = {
-      port: opts[:port].to_i,
-      proto: proto
-    }
-    sopts[:name] = opts[:name] if opts[:name]
-    sopts[:resource] = opts[:resource] if opts[:resource]
-    service = host.services.where(sopts).first_or_initialize
-
+    service = host.services.where(port: opts[:port].to_i, proto: proto).first_or_initialize
     ostate = service.state
     opts.each { |k,v|
       if (service.attribute_names.include?(k.to_s))
@@ -112,15 +93,8 @@ def report_service(opts)
         dlog("Unknown attribute for Service: #{k}")
       end
     }
-
     service.state ||= Msf::ServiceState::Open
     service.info  ||= ""
-    parents = process_service_chain(host, opts.delete(:parents)) if opts[:parents]
-    if parents
-      parents.each do |parent|
-        service.parents << parent if parent && !service.parents.include?(parent)
-      end
-    end
 
     begin
       framework.events.on_db_service(service) if service.new_record?
@@ -189,44 +163,4 @@ def update_service(opts)
     return service
   }
   end
-
-  def process_service_chain(host, services)
-    return if services.nil? || host.nil?
-    return unless services.is_a?(Hash) || services.is_a?(::Array)
-    return unless host.is_a?(Mdm::Host)
-
-    services = [services] unless services.is_a?(Array)
-    services.map do |service|
-      return unless service.is_a?(Hash)
-      return if service[:port].nil? || service[:proto].nil?
-
-      parents = nil
-      if service[:parents]&.any?
-        parents = process_service_chain(host, service[:parents])
-      end
-
-      service_info = {
-        port: service[:port].to_i,
-        proto: service[:proto].to_s.downcase,
-      }
-      service_info[:name] = service[:name].downcase if service[:name]
-      service_info[:resource] = service[:resource] if service[:resource]
-      service_obj = host.services.find_or_create_by(service_info)
-      if service_obj.id.nil?
-        elog("Failed to create service #{service_info.inspect} for host #{host.name} (#{host.address})")
-        return
-      end
-      service_obj.state ||= Msf::ServiceState::Open
-      service_obj.info = service[:info] ? service[:info] : ''
-
-      if parents
-        parents.each do |parent|
-          service_obj.parents << parent if parent && !service_obj.parents.include?(parent)
-        end
-      end
-
-      service_obj
-    end
-
-  end
 end

lib/msf/core/db_manager/session.rb

@@ -250,27 +250,18 @@ def infer_vuln_from_session(session, wspace)
         workspace: wspace,
       }
 
-      if session.exploit.respond_to?(:service_details) && session.exploit.service_details
-        service_details = session.exploit.service_details
-        service_name = service_details[:service_name]
-        port = service_details[:port]
-        if port.nil?
-          port = session.respond_to?(:target_port) && session.target_port ? session.target_port : session.exploit_datastore["RPORT"]
-        end
-        proto = service_details[:protocol]
-        vuln_info[:service] = host.services.find_or_create_by(name: service_name, port: port.to_i, proto: proto, state: 'open')
-      end
-      unless vuln_info[:service]
-        port = session.respond_to?(:target_port) && session.target_port ? session.target_port : session.exploit_datastore["RPORT"]
-        vuln_info[:service] = host.services.find_by_port(port.to_i) if port
-      end
+      port    = session.exploit_datastore["RPORT"]
+      service = (port ? host.services.find_by_port(port.to_i) : nil)
+
+      vuln_info[:service] = service if service
 
       vuln = report_vuln(vuln_info)
 
       attempt_info = {
         host: host,
         module: mod_fullname,
         refs: refs,
+        service: service,
         session_id: s.id,
         timestamp: Time.now.utc,
         username: session.username,

lib/msf/core/db_manager/vuln.rb

@@ -44,15 +44,11 @@ def find_vuln_by_details(details_map, host, service=nil)
     other_vulns.empty? ? nil : other_vulns.first
   end
 
-  def find_vuln_by_refs(refs, host, service = nil, cve_only = true, resource = nil)
+  def find_vuln_by_refs(refs, host, service = nil, cve_only = true)
     ref_ids = cve_only ? refs.find_all { |ref| ref.name.starts_with? 'CVE-'} : refs
     relation = host.vulns.joins(:refs)
     if !service.try(:id).nil?
-      if resource
-        return relation.where(service_id: service.try(:id), refs: { id: ref_ids}, resource: resource).first
-      else
-        return relation.where(service_id: service.try(:id), refs: { id: ref_ids}).first
-      end
+      return relation.where(service_id: service.try(:id), refs: { id: ref_ids}).first
     end
     return relation.where(refs: { id: ref_ids}).first
   end
@@ -84,20 +80,12 @@ def has_vuln?(name)
   # opts MUST contain
   # +:host+:: the host where this vulnerability resides
   # +:name+:: the friendly name for this vulnerability (title)
-  # +:workspace+:: the workspace to report this vulnerability in
   #
   # opts can contain
   # +:info+::   a human readable description of the vuln, free-form text
   # +:refs+::   an array of Ref objects or string names of references
   # +:details+:: a hash with :key pointed to a find criteria hash and the rest containing VulnDetail fields
   # +:sname+:: the name of the service this vulnerability relates to, used to associate it or create it.
-  # +:exploited_at+:: a timestamp indicating when this vulnerability was exploited, if applicable
-  # +:ref_ids+:: an array of reference IDs to associate with this vulnerability
-  # +:service+:: a Mdm::Service object or a Hash with service attributes to associate this vulnerability with
-  # +:port+:: the port number of the service this vulnerability relates to, if applicable
-  # +:proto+:: the transport layer protocol of the service this vulnerability relates to, if applicable
-  # +:details_match+:: a Mdm:VulnDetail with details related to this vulnerability
-  # +:resource+:: a resource hash to associate with this vulnerability, such as a URI or pipe name
   #
   def report_vuln(opts)
     return if not active
@@ -153,16 +141,7 @@ def report_vuln(opts)
     vuln = nil
 
     # Identify the associated service
-    service_opt = opts.delete(:service)
-    case service_opt
-    when Mdm::Service
-      service = service_opt
-    when Hash
-      service = report_service(service_opt.merge(workspace: wspace, host: host))
-    else
-      dlog("Skipping service since it is not a Hash or Mdm::Service: #{service.class}")
-      service = nil
-    end
+    service = opts.delete(:service)
 
     # Treat port zero as no service
     if service or opts[:port].to_i > 0
@@ -181,17 +160,9 @@ def report_vuln(opts)
           sname = opts[:proto]
         end
 
-        # If sname and proto are not provided, this will assign the first service
-        # registered in the database for this host with the given port and proto.
-        # This is likely to be the TCP service.
-        sopts = {
-          workspace: wspace,
-          host: host,
-          port: opts[:port].to_i,
-          proto: proto
-        }
-        sopts[:name] = sname if sname.present?
-        service = report_service(sopts)
+        services = host.services.where(port: opts[:port].to_i, proto: proto)
+        services = services.where(name: sname) if sname.present?
+        service = services.first_or_create
       end
 
       # Try to find an existing vulnerability with the same service & references
@@ -201,12 +172,8 @@ def report_vuln(opts)
       # prevent dupes of the same vuln found by both local patch and
       # service detection.
       if rids and rids.length > 0
-        if opts[:resource]
-          vuln = find_vuln_by_refs(rids, host, service, nil, opts[:resource])
-        else
-          vuln = find_vuln_by_refs(rids, host, service)
-        end
-        vuln.service = service if vuln && !vuln.service_id?
+        vuln = find_vuln_by_refs(rids, host, service)
+        vuln.service = service if vuln
       end
     else
       # Try to find an existing vulnerability with the same host & references
@@ -227,17 +194,9 @@ def report_vuln(opts)
     # No matches, so create a new vuln record
     unless vuln
       if service
-        if opts[:resource]
-          vuln = service.vulns.find_by(name: name, resource: opts[:resource])
-        else
-          vuln = service.vulns.find_by_name(name)
-        end
+        vuln = service.vulns.find_by_name(name)
       else
-        if opts[:resource]
-          vuln = host.vulns.find_by(name: name, resource: opts[:resource])
-        else
-          vuln = host.vulns.find_by_name(name)
-        end
+        vuln = host.vulns.find_by_name(name)
       end
 
       unless vuln
@@ -249,7 +208,6 @@ def report_vuln(opts)
         }
 
         vinf[:service_id] = service.id if service
-        vinf[:resource] = opts[:resource] if opts[:resource]
         vuln = Mdm::Vuln.create(vinf)
 
         begin

lib/msf/core/exploit.rb

@@ -49,7 +49,7 @@ module CompatDefaults
   # https://docs.metasploit.com/docs/development/developing-modules/guides/how-to-write-a-check-method.html
   #
   ##
-  class CheckCode < Struct.new(:code, :message, :reason, :details, :vuln)
+  class CheckCode < Struct.new(:code, :message, :reason, :details)
     # Do customization here because we need class constants and special
     # optional values and the block mode of Struct.new does not support that.
     #
@@ -77,8 +77,8 @@ def Appears(reason = nil, details: {})
         self.new('appears', reason, details: details)
       end
 
-      def Vulnerable(reason = nil, details: {}, vuln: {})
-        self.new('vulnerable', reason, details: details, vuln: vuln)
+      def Vulnerable(reason = nil, details: {})
+        self.new('vulnerable', reason, details: details)
       end
 
       def Unsupported(reason = nil, details: {})
@@ -100,7 +100,7 @@ def ===(other)
       other.is_a?(self.class) && self.code == other.code
     end
 
-    def initialize(code, reason, details: {}, vuln: {})
+    def initialize(code, reason, details: {})
       msg = case code
         when 'unknown';     'Cannot reliably check exploitability.'
         when 'safe';        'The target is not exploitable.'
@@ -111,7 +111,7 @@ def initialize(code, reason, details: {}, vuln: {})
         else
           ''
       end
-      super(code, "#{msg} #{reason}".strip, reason, details, vuln)
+      super(code, "#{msg} #{reason}".strip, reason, details)
     end
 
     #