Performance: Added mysql helper to do strategy: reset manually for mysql.

config/brakeman.ignore

@@ -460,46 +460,6 @@
       "confidence": "Medium",
       "note": "Admin configured RegExp"
     },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "a5818edfcce4a3860c36ce71d434d1d4dd91fe3cac9ac945c71e4e2932ffe6cc",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/ticket/search.rb",
-      "line": 203,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "Ticket.select(\"DISTINCT(tickets.id), #{::SqlHelper.new(:object => (self)).get_order_select(::SqlHelper.new(:object => (self)).get_sort_by(params, \"updated_at\"), ::SqlHelper.new(:object => (self)).get_order_by(params, \"desc\"), \"tickets.updated_at\")}\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Ticket::Search",
-        "method": "search"
-      },
-      "user_input": "::SqlHelper.new(:object => (self)).get_order_select(::SqlHelper.new(:object => (self)).get_sort_by(params, \"updated_at\"), ::SqlHelper.new(:object => (self)).get_order_by(params, \"desc\"), \"tickets.updated_at\")",
-      "confidence": "Medium",
-      "note": "SqlHelper does properly escape table and column names."
-    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "a5818edfcce4a3860c36ce71d434d1d4dd91fe3cac9ac945c71e4e2932ffe6cc",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/ticket/search.rb",
-      "line": 212,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "Ticket.select(\"DISTINCT(tickets.id), #{::SqlHelper.new(:object => (self)).get_order_select(::SqlHelper.new(:object => (self)).get_sort_by(params, \"updated_at\"), ::SqlHelper.new(:object => (self)).get_order_by(params, \"desc\"), \"tickets.updated_at\")}\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Ticket::Search",
-        "method": "search"
-      },
-      "user_input": "::SqlHelper.new(:object => (self)).get_order_select(::SqlHelper.new(:object => (self)).get_sort_by(params, \"updated_at\"), ::SqlHelper.new(:object => (self)).get_order_by(params, \"desc\"), \"tickets.updated_at\")",
-      "confidence": "Medium",
-      "note": "SqlHelper does properly escape table and column names."
-    },
     {
       "warning_type": "Dangerous Send",
       "warning_code": 23,
@@ -540,6 +500,26 @@
       "confidence": "Medium",
       "note": "ObjectLookup.by_id works as designed"
     },
+    {
+      "warning_type": "Command Injection",
+      "warning_code": 14,
+      "fingerprint": "be422b13e9cd280bc5ae570cd575777a4d48d8a53aed09bb59d1db85eee4927b",
+      "check_name": "Execute",
+      "message": "Possible command injection",
+      "file": "lib/mysql_strategy.rb",
+      "line": 64,
+      "link": "https://brakemanscanner.org/docs/warning_types/command_injection/",
+      "code": "system(\"mysqldump #{mysql_arguments} > #{backup_file}\", :exception => true)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "MysqlStrategy",
+        "method": "s(:self).backup"
+      },
+      "user_input": "mysql_arguments",
+      "confidence": "Medium",
+      "note": "Mysql arguments are internal / from config."
+    },
     {
       "warning_type": "Denial of Service",
       "warning_code": 76,
@@ -716,6 +696,26 @@
       "confidence": "Weak",
       "note": "Reflections come from the models themselves and are thus safe to use."
     },
+    {
+      "warning_type": "Command Injection",
+      "warning_code": 14,
+      "fingerprint": "fe15417756eed2c518c355309ee042b57df5f88a5410858dce3fa9fe9c893b84",
+      "check_name": "Execute",
+      "message": "Possible command injection",
+      "file": "lib/mysql_strategy.rb",
+      "line": 56,
+      "link": "https://brakemanscanner.org/docs/warning_types/command_injection/",
+      "code": "system(\"mysql #{mysql_arguments} < #{backup_file}\", :exception => true)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "MysqlStrategy",
+        "method": "s(:self).rollback"
+      },
+      "user_input": "mysql_arguments",
+      "confidence": "Medium",
+      "note": "Mysql arguments are internal / from config."
+    },
     {
       "warning_type": "Denial of Service",
       "warning_code": 76,
@@ -723,7 +723,7 @@
       "check_name": "RegexDoS",
       "message": "Model attribute used in regular expression",
       "file": "app/models/ticket.rb",
-      "line": 1612,
+      "line": 1577,
       "link": "https://brakemanscanner.org/docs/warning_types/denial_of_service/",
       "code": "/#{Setting.get(\"send_no_auto_response_reg_exp\")}/i",
       "render_path": null,
@@ -737,6 +737,6 @@
       "note": "Admin configured RegExp"
     }
   ],
-  "updated": "2021-07-20 13:22:48 +0200",
+  "updated": "2021-07-22 13:52:48 +0200",
-  "brakeman_version": "5.0.4"
+  "brakeman_version": "5.1.1"
 }

lib/mysql_strategy.rb

@@ -0,0 +1,73 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+require 'fileutils'
+require 'digest'
+
+class MysqlStrategy
+  def self.db?
+    ActiveRecord::Base.connection.instance_values['config'][:adapter] == 'mysql2'
+  end
+
+  def self.db_checksum
+    @@db_checksum ||= begin # rubocop:disable Style/ClassVars
+      files = Dir[ Rails.root.join('db/**/*') ].reject { |f| File.directory?(f) }
+      content = files.map { |f| File.read(f) }.join
+      Digest::MD5.hexdigest(content).to_s
+    end
+  end
+
+  def self.basepath
+    Rails.root.join("tmp/mysql_reset/#{db_checksum}/")
+  end
+
+  def self.backup_file
+    "#{basepath}db.sql"
+  end
+
+  def self.backup_exists?
+    File.exist?(backup_file)
+  end
+
+  def self.username
+    ActiveRecord::Base.connection.instance_values['config'][:username]
+  end
+
+  def self.password
+    ActiveRecord::Base.connection.instance_values['config'][:password]
+  end
+
+  def self.host
+    ActiveRecord::Base.connection.instance_values['config'][:host] || '127.0.0.1'
+  end
+
+  def self.database
+    ActiveRecord::Base.connection.instance_values['config'][:database]
+  end
+
+  def self.mysql_arguments
+    args = " -u#{username} -h#{host}"
+    args += " -p#{password}" if password.present?  # allow for passwordless access on dev systems
+    args + " #{database}"
+  end
+
+  def self.rollback
+    system("mysql #{mysql_arguments} < #{backup_file}", exception: true)
+    Rake::Task['zammad:db:rebuild'].reenable
+    Rake::Task['zammad:db:rebuild'].invoke
+  end
+
+  def self.backup
+    Rake::Task['zammad:db:reset'].reenable
+    Rake::Task['zammad:db:reset'].invoke
+    system("mysqldump #{mysql_arguments} > #{backup_file}", exception: true)
+  end
+
+  def self.reset
+    FileUtils.mkdir_p Rails.root.join(basepath)
+    if backup_exists?
+      rollback
+    else
+      backup
+    end
+  end
+end

lib/tasks/zammad/db/rebuild.rake

@@ -0,0 +1,16 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+namespace :zammad do
+
+  namespace :db do
+
+    desc 'Clears the Cache and reloads the Settings'
+    task rebuild: :environment do
+      Package::Migration.linked
+      ActiveRecord::Base.connection.reconnect!
+      ActiveRecord::Base.descendants.each(&:reset_column_information)
+      Cache.clear
+      Setting.reload
+    end
+  end
+end

lib/tasks/zammad/db/reset.rake

@@ -10,7 +10,7 @@ namespace :zammad do
       # we loop over each dependent task to be able to
       # execute them and their prerequisites multiple times (in tests)
       # there is no way in rake to achieve that
-      %w[db:drop:_unsafe db:create db:migrate db:seed].each do |task|
+      %w[db:drop:_unsafe db:create db:migrate db:seed zammad:db:rebuild].each do |task|
 
         if task == 'db:drop:_unsafe'
           # ensure all DB connections are closed before dropping the DB
@@ -26,12 +26,6 @@ namespace :zammad do
       ensure
         $stdout = STDOUT
       end
-
-      Package::Migration.linked
-      ActiveRecord::Base.connection.reconnect!
-      ActiveRecord::Base.descendants.each(&:reset_column_information)
-      Cache.clear
-      Setting.reload
     end
   end
 end

spec/support/db_strategies.rb

@@ -3,18 +3,17 @@
 RSpec.configure do |config|
 
   config.around(:each, db_strategy: :reset) do |example|
-    if ActiveRecord::Base.connection.instance_values['config'][:adapter] != 'postgresql'
+    if MysqlStrategy.db?
       self.use_transactional_tests = false
     end
     example.run
-    if ActiveRecord::Base.connection.instance_values['config'][:adapter] == 'postgresql'
+    if MysqlStrategy.db?
+      MysqlStrategy.reset
+    else
       Models.all.each_key do |model|
         model.connection.schema_cache.clear!
         model.reset_column_information
       end
-    else
-      Rake::Task['zammad:db:reset'].reenable
-      Rake::Task['zammad:db:reset'].invoke
     end
   end
 end