SQL injection false negative for connections on complex objects

[EQuincerot]

Background

Brakeman version: 5.2.1 Rails version: 6 and 7 Ruby version: 3.0.3p157

def not_detected_injection_risk(query)
  base_record = [ActiveRecord::Base].find {true}
  base_record.connection.exec_query("SELECT * where x = #{query}")
end

def detected_injection_risk(query)
  base_record = [ActiveRecord::Base].first
  base_record.connection.exec_query("SELECT * where x = #{query}")
end

Issue

The same SQL injection risk exists in both functions. The first one is not detected whereas the second one is correctly detected.

[presidentbeef]

This is really unusual code.

There are two bits to this:

1. Brakeman looks for connection calls on several potential targets, including ActiveRecord::Base.
2. Brakeman can pick the first item out of an array, but there's no way it's going to support calls to e.g. find with an arbitrary block.

We could assume that any connection call is going to be on an ActiveRecord model. That would introduce false positives, but maybe not very many.

But before we go down that road, it would be helpful to know if this was based on real code or not.

[EQuincerot]

Yes this is the kind of code we use in production. As we are using multiple database, we have for example this use case:

   DatabaseHelper.all_databases # returns Db1Record, Db2Record (abstract model classes with rules to connect to db1 or db2)
   DatabaseHelper.all_databases.find { |db| db.connection.tables.include?(table) }.connection.execute_query("VACUUM ANALYZE #{table_name}")

Rely on connection would work for us, I don't think that will cause false positives in our situation.