Skip to content

Commit

Permalink
add some security fixes for brakeman
Browse files Browse the repository at this point in the history
  • Loading branch information
fiveNinePlusR committed Sep 30, 2024
1 parent 9ecb1e2 commit 3859c13
Show file tree
Hide file tree
Showing 12 changed files with 168 additions and 18 deletions.
4 changes: 2 additions & 2 deletions app/controllers/api/v1/location_machine_xrefs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions app/models/location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions app/models/machine_score_xref.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ 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|
r = Region.find_by_name(name.downcase)
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
Expand Down
12 changes: 7 additions & 5 deletions app/models/suggested_location.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
2 changes: 1 addition & 1 deletion app/views/locations/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand Down
2 changes: 1 addition & 1 deletion app/views/locations/_locations.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down
27 changes: 27 additions & 0 deletions bin/bundle-audit
Original file line number Diff line number Diff line change
@@ -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")
27 changes: 27 additions & 0 deletions bin/bundler-audit
Original file line number Diff line number Diff line change
@@ -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")
94 changes: 94 additions & 0 deletions config/brakeman.ignore
Original file line number Diff line number Diff line change
@@ -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"
}
2 changes: 1 addition & 1 deletion config/initializers/cookies_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 3859c13

Please sign in to comment.