Merge pull request #2014 from FFederi/fix-polymorphic-name-false-positive

Fix polymorphic_name false positive

Author: presidentbeef

lib/brakeman/checks/check_sql.rb

@@ -600,7 +600,8 @@ 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, :uuid, :escape, :escape_string
+    :where_values_hash, :foreign_key, :uuid, :escape, :escape_string,
+    :polymorphic_name
   ]
 
   def ignore_methods_in_sql

test/apps/rails5.2/app/models/user.rb

@@ -23,4 +23,12 @@ def foreign_key_thing
 
     User.joins("INNER JOIN <complex join involving custom SQL and #{foreign_key} interpolation>")
   end
+
+  def polymorphic_name_joins
+    MediaFile.joins(
+      "JOIN #{table_name}
+        ON media_files.parent_type = '#{polymorphic_name}'
+        AND media_files.parent_id = #{table_name}.id"
+    )
+  end
 end

test/tests/rails52.rb

@@ -103,6 +103,19 @@ def test_sql_injection_foreign_key
       :user_input => s(:call, s(:call, nil, :reflect_on_association, s(:lit, :foos)), :foreign_key)
   end
 
+  def test_sql_injection_polymorphic_name
+    assert_no_warning :type => :warning,
+      :warning_code => 0,
+      :fingerprint => "76aa7211f677f4341babbadbed9d1c64c1cf3073303e94d565fe6cdb0124e4dc",
+      :warning_type => "SQL Injection",
+      :line => 30,
+      :message => /^Possible\ SQL\ injection/,
+      :confidence => 2,
+      :relative_path => "app/models/user.rb",
+      :code => s(:call, s(:const, :MediaFile), :joins, s(:dstr, "JOIN ", s(:evstr, s(:call, nil, :table_name)), s(:str, "\n        ON media_files.parent_type = '"), s(:evstr, s(:call, nil, :polymorphic_name)), s(:str, "'\n        AND media_files.parent_id = "), s(:evstr, s(:call, nil, :table_name)), s(:str, ".id"))),
+      :user_input => s(:call, nil, :polymorphic_name)
+  end
+
   def test_sql_injection_user_input
     assert_warning :type => :warning,
       :warning_code => 0,