From 5e495d72f5feb1360a95d382d90183d0db0a74e8 Mon Sep 17 00:00:00 2001
From: Jeffrey Martin <Jeffrey_Martin@rapid7.com>
Date: Tue, 13 Apr 2021 15:28:52 -0500
Subject: [PATCH] avoid side effects on arguments

When passed arguments as `opts` prefer to avoid side-effects
from method execution.

This extends similar work from #12740
---
 lib/msf/core/db_manager/client.rb        | 2 ++
 lib/msf/core/db_manager/host_tag.rb      | 1 +
 lib/msf/core/db_manager/loot.rb          | 1 +
 lib/msf/core/db_manager/payload.rb       | 1 +
 lib/msf/core/db_manager/report.rb        | 2 ++
 lib/msf/core/db_manager/service.rb       | 3 ++-
 lib/msf/core/db_manager/session.rb       | 1 +
 lib/msf/core/db_manager/session_event.rb | 1 +
 lib/msf/core/db_manager/user.rb          | 2 ++
 lib/msf/core/db_manager/vuln_attempt.rb  | 1 +
 lib/msf/core/db_manager/web.rb           | 4 ++++
 lib/msf/core/db_manager/workspace.rb     | 3 ++-
 12 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/msf/core/db_manager/client.rb b/lib/msf/core/db_manager/client.rb
index a41abb414d9d..d0ebcaf96c90 100644
--- a/lib/msf/core/db_manager/client.rb
+++ b/lib/msf/core/db_manager/client.rb
@@ -5,6 +5,7 @@ def find_or_create_client(opts)
 
   def get_client(opts)
   ::ApplicationRecord.connection_pool.with_connection {
+    opts = opts.clone() # protect the original caller's opts
     wspace = opts.delete(:workspace) || workspace
     host   = get_host(:workspace => wspace, :host => opts[:host]) || return
     client = host.clients.where({:ua_string => opts[:ua_string]}).first()
@@ -29,6 +30,7 @@ def get_client(opts)
   def report_client(opts)
     return if !active
   ::ApplicationRecord.connection_pool.with_connection {
+    opts = opts.clone() # protect the original caller's opts
     addr = opts.delete(:host) || return
     wspace = opts.delete(:workspace) || workspace
     report_host(:workspace => wspace, :host => addr)
diff --git a/lib/msf/core/db_manager/host_tag.rb b/lib/msf/core/db_manager/host_tag.rb
index 68f676783c39..b0033dfb253e 100644
--- a/lib/msf/core/db_manager/host_tag.rb
+++ b/lib/msf/core/db_manager/host_tag.rb
@@ -2,6 +2,7 @@ module Msf::DBManager::HostTag
   # This is only exercised by MSF3 XML importing for now. Needs the wait
   # conditions and return hash as well.
   def report_host_tag(opts)
+    opts = opts.clone() # protect the original caller's opts
     name = opts.delete(:name)
     raise Msf::DBImportError.new("Missing required option :name") unless name
     addr = opts.delete(:addr)
diff --git a/lib/msf/core/db_manager/loot.rb b/lib/msf/core/db_manager/loot.rb
index 18ef34c37831..661294f5f0ee 100644
--- a/lib/msf/core/db_manager/loot.rb
+++ b/lib/msf/core/db_manager/loot.rb
@@ -16,6 +16,7 @@ def loots(opts)
         return Array.wrap(Mdm::Loot.find(opts[:id]))
       end
 
+      opts = opts.clone() # protect the original caller's opts
       # Remove path from search conditions as this won't accommodate remote data
       # service usage where the client and server storage locations differ.
       opts.delete(:path)
diff --git a/lib/msf/core/db_manager/payload.rb b/lib/msf/core/db_manager/payload.rb
index 63abb98d11a2..d05eefed0c33 100644
--- a/lib/msf/core/db_manager/payload.rb
+++ b/lib/msf/core/db_manager/payload.rb
@@ -29,6 +29,7 @@ def payloads(opts)
 
   def update_payload(opts)
     ::ApplicationRecord.connection_pool.with_connection do
+      opts = opts.clone() # protect the original caller's opts
       id = opts.delete(:id)
       Mdm::Payload.update(id, opts)
     end
diff --git a/lib/msf/core/db_manager/report.rb b/lib/msf/core/db_manager/report.rb
index e869c0a775e3..20684ad4f755 100644
--- a/lib/msf/core/db_manager/report.rb
+++ b/lib/msf/core/db_manager/report.rb
@@ -20,6 +20,7 @@ def report_artifact(opts)
     tmp_path = opts[:file_path]
     artifact_name = File.basename tmp_path
     new_path = File.join(artifacts_dir, artifact_name)
+    opts = opts.clone() # protect the original caller's opts
     created = opts.delete(:created_at)
     updated = opts.delete(:updated_at)
 
@@ -55,6 +56,7 @@ def report_artifact(opts)
   # @return [Integer] ID of created report
   def report_report(opts)
     return if not active
+    opts = opts.clone() # protect the original caller's opts
     created = opts.delete(:created_at)
     updated = opts.delete(:updated_at)
     state   = opts.delete(:state)
diff --git a/lib/msf/core/db_manager/service.rb b/lib/msf/core/db_manager/service.rb
index 6e3ed11a3513..e0ee56b0a07e 100644
--- a/lib/msf/core/db_manager/service.rb
+++ b/lib/msf/core/db_manager/service.rb
@@ -51,12 +51,12 @@ def find_or_create_service(opts)
   def report_service(opts)
     return if !active
   ::ApplicationRecord.connection_pool.with_connection { |conn|
+    opts = opts.clone() # protect the original caller's opts
     addr  = opts.delete(:host) || return
     hname = opts.delete(:host_name)
     hmac  = opts.delete(:mac)
     host  = nil
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
-    opts = opts.clone()
     opts.delete(:workspace) # this may not be needed however the service creation below might complain if missing
     hopts = {:workspace => wspace, :host => addr}
     hopts[:name] = hname if hname
@@ -149,6 +149,7 @@ def services(opts)
       return Array.wrap(Mdm::Service.find(opts[:id]))
     end
 
+    opts = opts.clone() # protect the original caller's opts
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
     opts.delete(:workspace)
 
diff --git a/lib/msf/core/db_manager/session.rb b/lib/msf/core/db_manager/session.rb
index b1a720058ac9..25d73a6004a2 100644
--- a/lib/msf/core/db_manager/session.rb
+++ b/lib/msf/core/db_manager/session.rb
@@ -189,6 +189,7 @@ def update_session(opts)
     return if not active
 
     ::ApplicationRecord.connection_pool.with_connection {
+      opts = opts.clone() # protect the original caller's opts
       id = opts.delete(:id)
       session = ::Mdm::Session.find(id)
       session.update!(opts)
diff --git a/lib/msf/core/db_manager/session_event.rb b/lib/msf/core/db_manager/session_event.rb
index 5974f8584127..8d5d542f17d4 100644
--- a/lib/msf/core/db_manager/session_event.rb
+++ b/lib/msf/core/db_manager/session_event.rb
@@ -25,6 +25,7 @@ def session_events(opts)
         return Array.wrap(Mdm::SessionEvent.find(opts[:id]))
       end
 
+      opts = opts.clone() # protect the original caller's opts
       # Passing workspace keys to the search will cause exceptions, so remove them if they were accidentally included.
       opts.delete(:workspace)
 
diff --git a/lib/msf/core/db_manager/user.rb b/lib/msf/core/db_manager/user.rb
index ef8cd6607f1d..ca3596886af5 100644
--- a/lib/msf/core/db_manager/user.rb
+++ b/lib/msf/core/db_manager/user.rb
@@ -9,6 +9,7 @@ module Msf::DBManager::User
   def users(opts)
     ::ApplicationRecord.connection_pool.with_connection {
 
+      opts = opts.clone() # protect the original caller's opts
       search_term = opts.delete(:search_term)
       if search_term && !search_term.empty?
         column_search_conditions = Msf::Util::DBManager.create_all_column_search_conditions(Mdm::User, search_term)
@@ -74,6 +75,7 @@ def report_user(opts)
   # @return [Mdm::User] The updated Mdm::User object.
   def update_user(opts)
     ::ApplicationRecord.connection_pool.with_connection {
+      opts = opts.clone() # protect the original caller's opts
       id = opts.delete(:id)
       user = Mdm::User.find(id)
       user.update!(opts)
diff --git a/lib/msf/core/db_manager/vuln_attempt.rb b/lib/msf/core/db_manager/vuln_attempt.rb
index 12dec5e412d2..064198d14580 100644
--- a/lib/msf/core/db_manager/vuln_attempt.rb
+++ b/lib/msf/core/db_manager/vuln_attempt.rb
@@ -28,6 +28,7 @@ def vuln_attempts(opts)
       return Array.wrap(Mdm::VulnAttempt.find(opts[:id]))
     end
 
+    opts = opts.clone() # protect the original caller's opts
     # 'workspace' is not a valid attribute for Mdm::VulnAttempt. Remove it.
     opts.delete(:workspace)
 
diff --git a/lib/msf/core/db_manager/web.rb b/lib/msf/core/db_manager/web.rb
index d8a2410c2a3a..b4f0d5fef339 100644
--- a/lib/msf/core/db_manager/web.rb
+++ b/lib/msf/core/db_manager/web.rb
@@ -20,6 +20,7 @@ module Msf::DBManager::Web
   def report_web_form(opts)
     return if not active
   ::ApplicationRecord.connection_pool.with_connection {
+    opts = opts.clone() # protect the original caller's opts
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
 
     path    = opts[:path]
@@ -107,6 +108,7 @@ def report_web_form(opts)
   def report_web_page(opts)
     return if not active
   ::ApplicationRecord.connection_pool.with_connection {
+    opts = opts.clone() # protect the original caller's opts
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
 
     path    = opts[:path]
@@ -188,6 +190,7 @@ def report_web_page(opts)
   def report_web_site(opts)
     return if not active
   ::ApplicationRecord.connection_pool.with_connection { |conn|
+    opts = opts.clone() # protect the original caller's opts
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
     vhost  = opts.delete(:vhost)
 
@@ -289,6 +292,7 @@ def report_web_site(opts)
   def report_web_vuln(opts)
     return if not active
   ::ApplicationRecord.connection_pool.with_connection {
+    opts = opts.clone() # protect the original caller's opts
     wspace = Msf::Util::DBManager.process_opts_workspace(opts, framework)
 
     path    = opts[:path]
diff --git a/lib/msf/core/db_manager/workspace.rb b/lib/msf/core/db_manager/workspace.rb
index 0b925b0f69ef..19b2857a1dd2 100644
--- a/lib/msf/core/db_manager/workspace.rb
+++ b/lib/msf/core/db_manager/workspace.rb
@@ -52,7 +52,7 @@ def workspaces(opts = {})
       return Array.wrap(Mdm::Workspace.find(opts[:id]))
     end
 
-    opts = opts.clone() # protect the original callers array
+    opts = opts.clone() # protect the original caller's opts
     search_term = opts.delete(:search_term)
     # Passing these values to the search will cause exceptions, so remove them if they accidentally got passed in.
     opts.delete(:workspace)
@@ -95,6 +95,7 @@ def delete_workspaces(opts)
 
   def update_workspace(opts)
     raise ArgumentError.new("The following options are required: :id") if opts[:id].nil?
+    opts = opts.clone() # protect the original caller's opts
     opts.delete(:workspace)
 
     ::ApplicationRecord.connection_pool.with_connection {