Skip to content

Commit

Permalink
Merge pull request #1553 from presidentbeef/uuid_safe
Browse files Browse the repository at this point in the history
Treat UUIDs as safe values
  • Loading branch information
presidentbeef committed Jan 20, 2021
2 parents 26865c7 + 6d39f33 commit 09b299f
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/brakeman/checks/base_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def initialize(tracker)
@mass_assign_disabled = nil
@has_user_input = nil
@in_array = false
@safe_input_attributes = Set[:to_i, :to_f, :arel_table, :id]
@safe_input_attributes = Set[:to_i, :to_f, :arel_table, :id, :uuid]
@comparison_ops = Set[:==, :!=, :>, :<, :>=, :<=]
end

Expand Down
2 changes: 1 addition & 1 deletion lib/brakeman/checks/check_sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ def check_string_arg exp
:sanitize_sql_for_assignment, :sanitize_sql_for_conditions, :sanitize_sql_hash,
:sanitize_sql_hash_for_assignment, :sanitize_sql_hash_for_conditions,
:to_sql, :sanitize, :primary_key, :table_name_prefix, :table_name_suffix,
:where_values_hash, :foreign_key
:where_values_hash, :foreign_key, :uuid
]

def safe_value? exp
Expand Down
3 changes: 3 additions & 0 deletions test/apps/rails6/app/models/group.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
class Group < ApplicationRecord
def uuid_in_sql
ActiveRecord::Base.connection.exec_query("select * where x = #{User.uuid}")
end
end
13 changes: 13 additions & 0 deletions test/tests/rails6.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ def test_sql_injection_nonstandard_directory
:user_input => s(:lvar, :direction)
end

def test_sql_injection_uuid_false_positive
assert_no_warning :type => :warning,
:warning_code => 0,
:fingerprint => "8ebbafd2b7d6aa2d6ab639c6678ae1f5489dc3166747bbad8919e95156592321",
:warning_type => "SQL Injection",
:line => 3,
:message => /^Possible\ SQL\ injection/,
:confidence => 0,
:relative_path => "app/models/group.rb",
:code => s(:call, s(:call, s(:colon2, s(:const, :ActiveRecord), :Base), :connection), :exec_query, s(:dstr, "select * where x = ", s(:evstr, s(:call, s(:const, :User), :uuid)))),
:user_input => s(:call, s(:const, :User), :uuid)
end

def test_cross_site_scripting_sanity
assert_warning :type => :template,
:warning_code => 2,
Expand Down

0 comments on commit 09b299f

Please sign in to comment.