Support named :conditions parameter to some SQL executing ActiveRecord calls

ql/src/codeql_ruby/frameworks/ActiveRecord.qll

@@ -54,23 +54,29 @@ class ActiveRecordModelClassMethodCall extends MethodCall {
     this.getReceiver() instanceof ActiveRecordModelClassMethodCall
     or
     // e.g. self.where(...) within an ActiveRecordModelClass
-    (
-      this.getReceiver() instanceof Self
-      and
-      this.getEnclosingModule() instanceof ActiveRecordModelClass
-    )
+    this.getReceiver() instanceof Self and
+    this.getEnclosingModule() instanceof ActiveRecordModelClass
   }
 }
 
-private predicate methodWithSqlFragmentArg(string methodName, int argIndex) {
-  methodName =
-    [
-      "delete_all", "destroy_all", "exists?", "find_by", "find_by_sql", "from", "group", "having",
-      "joins", "lock", "not", "order", "pluck", "where"
-    ] and
-  argIndex = 0
-  or
-  methodName = "calculate" and argIndex = 1
+private Expr sqlFragmentArgument(MethodCall call) {
+  exists(string methodName |
+    methodName = call.getMethodName() and
+    (
+      methodName =
+        [
+          "delete_all", "destroy_all", "exists?", "find_by", "find_by_sql", "from", "group",
+          "having", "joins", "lock", "not", "order", "pluck", "where"
+        ] and
+      result = call.getArgument(0)
+      or
+      methodName = "calculate" and result = call.getArgument(1)
+      or
+      // This format was supported until Rails 2.3.8
+      methodName = ["all", "find", "first", "last"] and
+      result = call.getKeywordArgument("conditions")
+    )
+  )
 }
 
 // An expression that, if tainted by unsanitized input, should not be used as
@@ -113,10 +119,6 @@ private predicate unsafeSqlArg(Expr sqlFragmentExpr) {
  * rather than just one with a matching name.
  */
 class PotentiallyUnsafeSqlExecutingMethodCall extends ActiveRecordModelClassMethodCall {
-  // The name of the method invoked
-  private string methodName;
-  // The zero-indexed position of the SQL fragment sink argument
-  private int sqlFragmentArgumentIndex;
   // The SQL fragment argument itself
   private Expr sqlFragmentExpr;
 
@@ -124,9 +126,7 @@ class PotentiallyUnsafeSqlExecutingMethodCall extends ActiveRecordModelClassMeth
   // TODO: refine this further to account for cases where the method called has
   //       been overriden to perform validation on its arguments
   PotentiallyUnsafeSqlExecutingMethodCall() {
-    methodName = this.getMethodName() and
-    sqlFragmentExpr = this.getArgument(sqlFragmentArgumentIndex) and
-    methodWithSqlFragmentArg(methodName, sqlFragmentArgumentIndex) and
+    sqlFragmentExpr = sqlFragmentArgument(this) and
     unsafeSqlArg(sqlFragmentExpr)
   }