Ruby: Track flow into ActiveRecord scopes

Author: hmac

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll

@@ -429,7 +429,20 @@ private Callable viableSourceCallableInit(RelevantCall call) { result = getIniti
 /** Holds if `call` may resolve to the returned source-code method. */
 private DataFlowCallable viableSourceCallable(DataFlowCall call) {
   result = viableSourceCallableNonInit(call) or
-  result.asCfgScope() = viableSourceCallableInit(call.asCall())
+  result.asCfgScope() = viableSourceCallableInit(call.asCall()) or
+  result = any(AdditionalCallTarget t).viableTarget(call.asCall())
+}
+
+/**
+ * A unit class for adding additional call steps.
+ *
+ * Extend this class to add additional call steps to the data flow graph.
+ */
+class AdditionalCallTarget extends Unit {
+  /**
+   * Gets a viable target for `call`.
+   */
+  abstract DataFlowCallable viableTarget(CfgNodes::ExprNodes::CallCfgNode call);
 }
 
 /** Holds if `call` may resolve to the returned summarized library method. */

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

@@ -765,3 +765,30 @@ private class ActiveRecordCollectionProxyModelInstantiation extends ActiveRecord
     result = this.(ActiveRecordCollectionProxyMethodCall).getAssociation().getTargetClass()
   }
 }
+
+/**
+ * An additional call step for calls to ActiveRecord scopes. For example, in the following code:
+ *
+ * ```rb
+ * class User < ActiveRecord::Base
+ *   scope :with_role, ->(role) { where(role: role) }
+ * end
+ *
+ * User.with_role(r)
+ * ```
+ *
+ * the call to `with_role` targets the lambda, and argument `r` flows to the parameter `role`.
+ */
+class ActiveRecordScopeCallTarget extends AdditionalCallTarget {
+  override DataFlowCallable viableTarget(ExprNodes::CallCfgNode scopeCall) {
+    exists(DataFlow::ModuleNode model, string scopeName |
+      model = activeRecordBaseClass().getADescendentModule() and
+      exists(DataFlow::CallNode scope |
+        scope = model.getAModuleLevelCall("scope") and
+        scope.getArgument(0).getConstantValue().isStringlikeValue(scopeName) and
+        scope.getArgument(1).asCallable().asCallableAstNode() = result.asCfgScope()
+      ) and
+      scopeCall = model.getAnImmediateReference().getAMethodCall(scopeName).asExpr()
+    )
+  }
+}

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

@@ -204,3 +204,14 @@ def show
     Regression.connection.execute("SELECT * FROM users WHERE id = #{permitted_params[:user_id]}")
   end
 end
+
+class User
+  scope :with_role, ->(role) { where("role = #{role}") }
+end
+
+class UsersController < ActionController::Base
+  def index
+    # BAD: user input passed to scope which uses it without sanitization.
+    @users = User.with_role(params[:role])
+  end
+end

ruby/ql/test/query-tests/security/cwe-089/SqlInjection.expected

@@ -76,6 +76,9 @@ edges
 | ActiveRecordInjection.rb:203:77:203:102 | ...[...] | ActiveRecordInjection.rb:203:43:203:104 | "SELECT * FROM users WHERE id ..." | provenance |  |
 | ActiveRecordInjection.rb:204:69:204:84 | call to permitted_params | ActiveRecordInjection.rb:204:69:204:94 | ...[...] | provenance |  |
 | ActiveRecordInjection.rb:204:69:204:94 | ...[...] | ActiveRecordInjection.rb:204:35:204:96 | "SELECT * FROM users WHERE id ..." | provenance |  |
+| ActiveRecordInjection.rb:209:24:209:27 | role | ActiveRecordInjection.rb:209:38:209:53 | "role = #{...}" | provenance |  |
+| ActiveRecordInjection.rb:215:29:215:34 | call to params | ActiveRecordInjection.rb:215:29:215:41 | ...[...] | provenance |  |
+| ActiveRecordInjection.rb:215:29:215:41 | ...[...] | ActiveRecordInjection.rb:209:24:209:27 | role | provenance |  |
 | ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | provenance |  |
 | ArelInjection.rb:4:5:4:8 | name | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | provenance |  |
 | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:4:12:4:29 | ...[...] | provenance |  |
@@ -201,6 +204,10 @@ nodes
 | ActiveRecordInjection.rb:204:35:204:96 | "SELECT * FROM users WHERE id ..." | semmle.label | "SELECT * FROM users WHERE id ..." |
 | ActiveRecordInjection.rb:204:69:204:84 | call to permitted_params | semmle.label | call to permitted_params |
 | ActiveRecordInjection.rb:204:69:204:94 | ...[...] | semmle.label | ...[...] |
+| ActiveRecordInjection.rb:209:24:209:27 | role | semmle.label | role |
+| ActiveRecordInjection.rb:209:38:209:53 | "role = #{...}" | semmle.label | "role = #{...}" |
+| ActiveRecordInjection.rb:215:29:215:34 | call to params | semmle.label | call to params |
+| ActiveRecordInjection.rb:215:29:215:41 | ...[...] | semmle.label | ...[...] |
 | ArelInjection.rb:4:5:4:8 | name | semmle.label | name |
 | ArelInjection.rb:4:12:4:17 | call to params | semmle.label | call to params |
 | ArelInjection.rb:4:12:4:29 | ...[...] | semmle.label | ...[...] |
@@ -257,6 +264,7 @@ subpaths
 | ActiveRecordInjection.rb:194:37:194:41 | query | ActiveRecordInjection.rb:199:5:199:10 | call to params | ActiveRecordInjection.rb:194:37:194:41 | query | This SQL query depends on a $@. | ActiveRecordInjection.rb:199:5:199:10 | call to params | user-provided value |
 | ActiveRecordInjection.rb:203:43:203:104 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:199:5:199:10 | call to params | ActiveRecordInjection.rb:203:43:203:104 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:199:5:199:10 | call to params | user-provided value |
 | ActiveRecordInjection.rb:204:35:204:96 | "SELECT * FROM users WHERE id ..." | ActiveRecordInjection.rb:199:5:199:10 | call to params | ActiveRecordInjection.rb:204:35:204:96 | "SELECT * FROM users WHERE id ..." | This SQL query depends on a $@. | ActiveRecordInjection.rb:199:5:199:10 | call to params | user-provided value |
+| ActiveRecordInjection.rb:209:38:209:53 | "role = #{...}" | ActiveRecordInjection.rb:215:29:215:34 | call to params | ActiveRecordInjection.rb:209:38:209:53 | "role = #{...}" | This SQL query depends on a $@. | ActiveRecordInjection.rb:215:29:215:34 | call to params | user-provided value |
 | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:6:20:6:61 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |
 | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | ArelInjection.rb:4:12:4:17 | call to params | ArelInjection.rb:7:39:7:80 | "SELECT * FROM users WHERE nam..." | This SQL query depends on a $@. | ArelInjection.rb:4:12:4:17 | call to params | user-provided value |
 | PgInjection.rb:14:15:14:18 | qry1 | PgInjection.rb:6:12:6:17 | call to params | PgInjection.rb:14:15:14:18 | qry1 | This SQL query depends on a $@. | PgInjection.rb:6:12:6:17 | call to params | user-provided value |