Merge pull request #15857 from joefarebrother/ruby-activerecord-from

Ruby: Model second argument of `ActiveRecord` `from`

Author: joefarebrother

ruby/ql/lib/change-notes/2024-03-08-activerecord-from.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The second argument, `subquery_name`, of the `ActiveRecord::QueryMethods::from` method, is now recognized as an sql injection sink.

ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll

@@ -175,14 +175,14 @@ private predicate sqlFragmentArgumentInner(DataFlow::CallNode call, DataFlow::No
   call =
     activeRecordQueryBuilderCall([
         "delete_all", "delete_by", "destroy_all", "destroy_by", "exists?", "find_by", "find_by!",
-        "find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "from",
-        "having", "lock", "not", "where", "rewhere"
+        "find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "find_by_sql", "having",
+        "lock", "not", "where", "rewhere"
       ]) and
   sink = call.getArgument(0)
   or
   call =
     activeRecordQueryBuilderCall([
-        "group", "joins", "order", "reorder", "pluck", "select", "reselect"
+        "from", "group", "joins", "order", "reorder", "pluck", "select", "reselect"
       ]) and
   sink = call.getArgument(_)
   or

ruby/ql/test/query-tests/security/cwe-089/ActiveRecordInjection.rb

@@ -114,6 +114,12 @@ def some_request_handler
     User.joins(:a, params[:column])
 
     User.count_by_sql(params[:custom_sql_query])
+
+    # BAD: executes `SELECT users.* FROM #{params[:tab]}`
+    # where `params[:tab]` is unsanitized
+    User.all.from(params[:tab]) 
+    # BAD: executes `SELECT "users".* FROM (SELECT "users".* FROM "users") #{params[:sq]}
+    User.all.from(User.all, params[:sq])
   end
 end