From 3859c13ffda6930cb65f2d6dd2f09085266d9bee Mon Sep 17 00:00:00 2001
From: fiveNinePlusR <fiveNinePlusR@gmail.com>
Date: Sun, 29 Sep 2024 19:58:37 -0700
Subject: [PATCH] add some security fixes for brakeman

---
 .../v1/location_machine_xrefs_controller.rb   |  4 +-
 .../v1/location_picture_xrefs_controller.rb   |  2 +-
 app/models/location.rb                        |  4 +-
 app/models/machine_score_xref.rb              |  8 +-
 app/models/suggested_location.rb              | 12 ++-
 app/models/user.rb                            |  2 +-
 app/views/locations/_form.html.haml           |  2 +-
 app/views/locations/_locations.html.haml      |  2 +-
 bin/bundle-audit                              | 27 ++++++
 bin/bundler-audit                             | 27 ++++++
 config/brakeman.ignore                        | 94 +++++++++++++++++++
 config/initializers/cookies_serializer.rb     |  2 +-
 12 files changed, 168 insertions(+), 18 deletions(-)
 create mode 100755 bin/bundle-audit
 create mode 100755 bin/bundler-audit
 create mode 100644 config/brakeman.ignore

diff --git a/app/controllers/api/v1/location_machine_xrefs_controller.rb b/app/controllers/api/v1/location_machine_xrefs_controller.rb
index f55a90568..fa2724b3a 100644
--- a/app/controllers/api/v1/location_machine_xrefs_controller.rb
+++ b/app/controllers/api/v1/location_machine_xrefs_controller.rb
@@ -38,8 +38,8 @@ def create
 
         return return_response(AUTH_REQUIRED_MSG, 'errors') if user.nil?
 
-        location_id = params[:location_id]
-        machine_id = params[:machine_id]
+        location_id = params[:location_id].to_i
+        machine_id = params[:machine_id].to_i
         condition = params[:condition]
         status_code = 200
 
diff --git a/app/controllers/api/v1/location_picture_xrefs_controller.rb b/app/controllers/api/v1/location_picture_xrefs_controller.rb
index 867a831f3..82ca86890 100644
--- a/app/controllers/api/v1/location_picture_xrefs_controller.rb
+++ b/app/controllers/api/v1/location_picture_xrefs_controller.rb
@@ -21,7 +21,7 @@ def show
       def create
         return return_response(AUTH_REQUIRED_MSG, 'errors') if current_user.nil?
 
-        location_id = params[:location_id]
+        location_id = params[:location_id].to_i
         return return_response('Failed to find location', 'errors') if location_id.nil? || !Location.exists?(location_id)
 
         photo = params[:photo]
diff --git a/app/models/location.rb b/app/models/location.rb
index e14302fd3..e3352c82d 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -3,8 +3,8 @@ class Location < ApplicationRecord
 
   validates_presence_of :name, :street, :city, :country
   validates :phone, phone: { possible: true, allow_blank: true, message: 'Phone format not valid.' }
-  validates :website, format: { with: %r{http(s?)://}, message: 'must begin with http:// or https://' }, if: :website?
-  validates :name, :street, :city, format: { with: /^\S.*/, message: "Can't start with a blank", multiline: true }
+  validates :website, format: { with: %r{\Ahttp(s?)://}, message: 'must begin with http:// or https://' }, if: :website?
+  validates :name, :street, :city, format: { with: /\A\S.*/, message: "Can't start with a blank", multiline: true }
   validates :lat, :lon, presence: { message: 'Latitude/Longitude failed to generate. Please double check address and try again, or manually enter the lat/lon' }
 
   belongs_to :location_type, optional: true
diff --git a/app/models/machine_score_xref.rb b/app/models/machine_score_xref.rb
index c55b44955..6f17986a0 100644
--- a/app/models/machine_score_xref.rb
+++ b/app/models/machine_score_xref.rb
@@ -8,8 +8,8 @@ class MachineScoreXref < ApplicationRecord
 
   scope :zone_id, lambda { |id|
     joins(:location_machine_xref).joins(:location).where("
-      locations.zone_id = #{id}
-    ")
+      locations.zone_id = ?
+    ", id)
   }
 
   scope :region, lambda { |name|
@@ -17,8 +17,8 @@ class MachineScoreXref < ApplicationRecord
     joins(:location_machine_xref).joins(:location).where("
       location_machine_xrefs.id = machine_score_xrefs.location_machine_xref_id
       and locations.id = location_machine_xrefs.location_id
-      and locations.region_id = #{r.id}
-    ")
+      and locations.region_id = ?
+    ", r.id)
   }
 
   def username
diff --git a/app/models/suggested_location.rb b/app/models/suggested_location.rb
index 98e4fced1..d62fb5850 100644
--- a/app/models/suggested_location.rb
+++ b/app/models/suggested_location.rb
@@ -5,8 +5,8 @@ class SuggestedLocation < ApplicationRecord
   validates_presence_of :name, :machines, on: :create
   validates_presence_of :street, :city, :zip, on: :update
 
-  validates :website, format: { with: %r{http(s?)://}, message: 'must begin with http:// or https://' }, if: :website?, on: :update
-  validates :name, :street, :city, format: { with: /^\S.*/, message: "Can't start with a blank", multiline: true }, on: :update
+  validates :website, format: { with: %r{\Ahttp(s?)://}, message: 'must begin with http:// or https://' }, if: :website?, on: :update
+  validates :name, :street, :city, format: { with: /\A\S.*/, message: "Can't start with a blank", multiline: true }, on: :update
   validates :lat, :lon, presence: { message: 'Latitude/Longitude failed to generate. Please double check address and try again, or manually enter the lat/lon' }, on: :update
 
   belongs_to :region, optional: true
@@ -97,19 +97,21 @@ def convert_to_location(user_email)
 
       delete
 
-      ActiveRecord::Base.connection.execute(<<HERE)
+      sql = <<HERE
 insert into versions values (
   nextval('versions_id_seq'),
   'Location',
-  #{location.id},
+  :location_id,
   'converted from suggested location',
-  '#{user_email}',
+  :user_email,
   NULL,
   now(),
   NULL,
   NULL
 )
 HERE
+      sanitized_sql = ActiveRecord::Base.sanitize_sql_array([sql, { location_id: location.id, user_email: user_email }])
+      ActiveRecord::Base.connection.execute(sanitized_sql)
     end
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 3cfa1537d..aa3b40da7 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -11,7 +11,7 @@ class User < ApplicationRecord
 
   validates :username, presence: true, uniqueness: { case_sensitive: false }
 
-  validates_format_of :username, with: /^[a-zA-Z0-9_\.]*$/, multiline: true
+  validates_format_of :username, with: /\A[a-zA-Z0-9_\.]*\z/, multiline: true
   validates :username, length: { maximum: 20 }
   strip_attributes only: %i[username password]
 
diff --git a/app/views/locations/_form.html.haml b/app/views/locations/_form.html.haml
index 59f2c18a7..a8b42f0b2 100644
--- a/app/views/locations/_form.html.haml
+++ b/app/views/locations/_form.html.haml
@@ -32,7 +32,7 @@
         :javascript
           $(function () {
             $('#by_#{key}_name').autocomplete({
-              source: '/#{key}s/autocomplete?region_level_search=1;region=#{params[:region]}',
+              source: '/#{key}s/autocomplete?region_level_search=1;region=#{h(params[:region])}',
               minLength: 2,
               delay: 500
               });
diff --git a/app/views/locations/_locations.html.haml b/app/views/locations/_locations.html.haml
index 2bc1bdfa6..d42cfc60d 100644
--- a/app/views/locations/_locations.html.haml
+++ b/app/views/locations/_locations.html.haml
@@ -15,7 +15,7 @@
       [#{[@lat, @lon].join(', ')}]
     );
 
-    var hrefOrig = '#{request.scheme}://#{request.host_with_port}/#{@region ? @region.name.downcase : @operators_map ? @operators_map : "map"}?';
+    var hrefOrig = '#{request.scheme}://#{request.host_with_port}/#{@region ? h(@region.name.downcase) : @operators_map ? @operators_map : "map"}?';
     var def_value = window.location.href;
 
     var url = '';
diff --git a/bin/bundle-audit b/bin/bundle-audit
new file mode 100755
index 000000000..a0e7ba0e8
--- /dev/null
+++ b/bin/bundle-audit
@@ -0,0 +1,27 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+#
+# This file was generated by Bundler.
+#
+# The application 'bundle-audit' is installed as part of a gem, and
+# this file is here to facilitate running it.
+#
+
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)
+
+bundle_binstub = File.expand_path("bundle", __dir__)
+
+if File.file?(bundle_binstub)
+  if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
+    load(bundle_binstub)
+  else
+    abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
+Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
+  end
+end
+
+require "rubygems"
+require "bundler/setup"
+
+load Gem.bin_path("bundler-audit", "bundle-audit")
diff --git a/bin/bundler-audit b/bin/bundler-audit
new file mode 100755
index 000000000..334a73784
--- /dev/null
+++ b/bin/bundler-audit
@@ -0,0 +1,27 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+#
+# This file was generated by Bundler.
+#
+# The application 'bundler-audit' is installed as part of a gem, and
+# this file is here to facilitate running it.
+#
+
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__)
+
+bundle_binstub = File.expand_path("bundle", __dir__)
+
+if File.file?(bundle_binstub)
+  if File.read(bundle_binstub, 300).include?("This file was generated by Bundler")
+    load(bundle_binstub)
+  else
+    abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
+Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
+  end
+end
+
+require "rubygems"
+require "bundler/setup"
+
+load Gem.bin_path("bundler-audit", "bundler-audit")
diff --git a/config/brakeman.ignore b/config/brakeman.ignore
new file mode 100644
index 000000000..b5e503e7c
--- /dev/null
+++ b/config/brakeman.ignore
@@ -0,0 +1,94 @@
+{
+  "ignored_warnings": [
+    {
+      "warning_type": "Format Validation",
+      "warning_code": 30,
+      "fingerprint": "06f9be88a8af832b07095516dcafa96909e3f7770c63c803906d29d94ed9f5fc",
+      "check_name": "ValidationRegex",
+      "message": "Insufficient validation for `website` using `/\\Ahttp(s?):\\/\\//`. Use `\\A` and `\\z` as anchors",
+      "file": "app/models/location.rb",
+      "line": 6,
+      "link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
+      "code": null,
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "Location"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        777
+      ],
+      "note": ""
+    },
+    {
+      "warning_type": "Format Validation",
+      "warning_code": 30,
+      "fingerprint": "06f9be88a8af832b07095516dcafa96909e3f7770c63c803906d29d94ed9f5fc",
+      "check_name": "ValidationRegex",
+      "message": "Insufficient validation for `name` using `/\\A\\S.*/`. Use `\\A` and `\\z` as anchors",
+      "file": "app/models/location.rb",
+      "line": 7,
+      "link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
+      "code": null,
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "Location"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        777
+      ],
+      "note": ""
+    },
+    {
+      "warning_type": "Format Validation",
+      "warning_code": 30,
+      "fingerprint": "8b632b14ee9e862130a926be342087e1f1ec9d174f244c2d61fc2a4514afd2f7",
+      "check_name": "ValidationRegex",
+      "message": "Insufficient validation for `website` using `/\\Ahttp(s?):\\/\\//`. Use `\\A` and `\\z` as anchors",
+      "file": "app/models/suggested_location.rb",
+      "line": 8,
+      "link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
+      "code": null,
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "SuggestedLocation"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        777
+      ],
+      "note": ""
+    },
+    {
+      "warning_type": "Format Validation",
+      "warning_code": 30,
+      "fingerprint": "8b632b14ee9e862130a926be342087e1f1ec9d174f244c2d61fc2a4514afd2f7",
+      "check_name": "ValidationRegex",
+      "message": "Insufficient validation for `name` using `/\\A\\S.*/`. Use `\\A` and `\\z` as anchors",
+      "file": "app/models/suggested_location.rb",
+      "line": 9,
+      "link": "https://brakemanscanner.org/docs/warning_types/format_validation/",
+      "code": null,
+      "render_path": null,
+      "location": {
+        "type": "model",
+        "model": "SuggestedLocation"
+      },
+      "user_input": null,
+      "confidence": "High",
+      "cwe_id": [
+        777
+      ],
+      "note": ""
+    }
+  ],
+  "updated": "2024-09-29 19:42:12 -0700",
+  "brakeman_version": "6.2.1"
+}
diff --git a/config/initializers/cookies_serializer.rb b/config/initializers/cookies_serializer.rb
index f51a497e1..5a6a32d37 100644
--- a/config/initializers/cookies_serializer.rb
+++ b/config/initializers/cookies_serializer.rb
@@ -2,4 +2,4 @@
 
 # Specify a serializer for the signed and encrypted cookie jars.
 # Valid options are :json, :marshal, and :hybrid.
-Rails.application.config.action_dispatch.cookies_serializer = :hybrid
+Rails.application.config.action_dispatch.cookies_serializer = :json