Ruby: Add OrmWriteAccess concept to model writes to a DB using an ORM

ruby/ql/lib/change-notes/2022-02-28-orm-write-access.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Added `OrmWriteAccess` concept to model data written to a database using an object-relational mapping (ORM) library.

ruby/ql/lib/codeql/ruby/Concepts.qll

@@ -625,6 +625,35 @@ module OrmInstantiation {
   }
 }
 
+/**
+ * A data flow node that writes persistent data.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `PersistentWriteAccess::Range` instead.
+ */
+class PersistentWriteAccess extends DataFlow::Node instanceof PersistentWriteAccess::Range {
+  /**
+   * Gets the data flow node corresponding to the written value.
+   */
+  DataFlow::Node getValue() { result = super.getValue() }
+}
+
+/** Provides a class for modeling new persistent write access APIs. */
+module PersistentWriteAccess {
+  /**
+   * A data flow node that writes persistent data.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `PersistentWriteAccess` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /**
+     * Gets the data flow node corresponding to the written value.
+     */
+    abstract DataFlow::Node getValue();
+  }
+}
+
 /**
  * A data-flow node that may set or unset Cross-site request forgery protection.
  *

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

@@ -314,3 +314,151 @@ private class ActiveRecordInstanceMethodCall extends DataFlow::CallNode {
 
   ActiveRecordInstance getInstance() { result = instance }
 }
+
+/**
+ * Provides modeling relating to the `ActiveRecord::Persistence` module.
+ */
+private module Persistence {
+  /**
+   * Holds if there is a hash literal argument to `call` at `argIndex`
+   * containing a KV pair with value `value`.
+   */
+  private predicate hashArgumentWithValue(
+    DataFlow::CallNode call, int argIndex, DataFlow::ExprNode value
+  ) {
+    exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
+      hash = call.getArgument(argIndex).asExpr() and
+      pair = hash.getAKeyValuePair()
+    |
+      value.asExpr() = pair.getValue()
+    )
+  }
+
+  /**
+   * Holds if `call` has a keyword argument of with value `value`.
+   */
+  private predicate keywordArgumentWithValue(DataFlow::CallNode call, DataFlow::ExprNode value) {
+    exists(ExprNodes::PairCfgNode pair | pair = call.getArgument(_).asExpr() |
+      value.asExpr() = pair.getValue()
+    )
+  }
+
+  /** A call to e.g. `User.create(name: "foo")` */
+  private class CreateLikeCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
+    private ActiveRecordModelClass modelCls;
+
+    CreateLikeCall() {
+      modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
+      this.getMethodName() =
+        [
+          "create", "create!", "create_or_find_by", "create_or_find_by!", "find_or_create_by",
+          "find_or_create_by!", "insert", "insert!"
+        ]
+    }
+
+    override DataFlow::Node getValue() {
+      // attrs as hash elements in arg0
+      hashArgumentWithValue(this, 0, result) or
+      keywordArgumentWithValue(this, result)
+    }
+  }
+
+  /** A call to e.g. `User.update(1, name: "foo")` */
+  private class UpdateLikeClassMethodCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
+    private ActiveRecordModelClass modelCls;
+
+    UpdateLikeClassMethodCall() {
+      modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
+      this.getMethodName() = ["update", "update!", "upsert"]
+    }
+
+    override DataFlow::Node getValue() {
+      keywordArgumentWithValue(this, result)
+      or
+      // Case where 2 array args are passed - the first an array of IDs, and the
+      // second an array of hashes - each hash corresponding to an ID in the
+      // first array.
+      exists(ExprNodes::ArrayLiteralCfgNode hashesArray |
+        this.getArgument(0).asExpr() instanceof ExprNodes::ArrayLiteralCfgNode and
+        hashesArray = this.getArgument(1).asExpr()
+      |
+        exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
+          hash = hashesArray.getArgument(_) and
+          pair = hash.getAKeyValuePair()
+        |
+          result.asExpr() = pair.getValue()
+        )
+      )
+    }
+  }
+
+  /** A call to e.g. `User.insert_all([{name: "foo"}, {name: "bar"}])` */
+  private class InsertAllLikeCall extends DataFlow::CallNode, PersistentWriteAccess::Range {
+    private ExprNodes::ArrayLiteralCfgNode arr;
+    private ActiveRecordModelClass modelCls;
+
+    InsertAllLikeCall() {
+      modelCls = this.asExpr().getExpr().(ActiveRecordModelClassMethodCall).getReceiverClass() and
+      this.getMethodName() = ["insert_all", "insert_all!", "upsert_all"] and
+      arr = this.getArgument(0).asExpr()
+    }
+
+    override DataFlow::Node getValue() {
+      // attrs as hash elements of members of array arg0
+      exists(ExprNodes::HashLiteralCfgNode hash, ExprNodes::PairCfgNode pair |
+        hash = arr.getArgument(_) and
+        pair = hash.getAKeyValuePair()
+      |
+        result.asExpr() = pair.getValue()
+      )
+    }
+  }
+
+  /** A call to e.g. `user.update(name: "foo")` */
+  private class UpdateLikeInstanceMethodCall extends PersistentWriteAccess::Range,
+    ActiveRecordInstanceMethodCall {
+    UpdateLikeInstanceMethodCall() {
+      this.getMethodName() = ["update", "update!", "update_attributes", "update_attributes!"]
+    }
+
+    override DataFlow::Node getValue() {
+      // attrs as hash elements in arg0
+      hashArgumentWithValue(this, 0, result)
+      or
+      // keyword arg
+      keywordArgumentWithValue(this, result)
+    }
+  }
+
+  /** A call to e.g. `user.update_attribute(name, "foo")` */
+  private class UpdateAttributeCall extends PersistentWriteAccess::Range,
+    ActiveRecordInstanceMethodCall {
+    UpdateAttributeCall() { this.getMethodName() = "update_attribute" }
+
+    override DataFlow::Node getValue() {
+      // e.g. `foo.update_attribute(key, value)`
+      result = this.getArgument(1)
+    }
+  }
+
+  /**
+   * An assignment like `user.name = "foo"`. Though this does not write to the
+   * database without a subsequent call to persist the object, it is considered
+   * as an `PersistentWriteAccess` to avoid missing cases where the path to a
+   * subsequent write is not clear.
+   */
+  private class AssignAttribute extends PersistentWriteAccess::Range {
+    private ExprNodes::AssignExprCfgNode assignNode;
+
+    AssignAttribute() {
+      exists(DataFlow::CallNode setter |
+        assignNode = this.asExpr() and
+        setter.getArgument(0) = this and
+        setter instanceof ActiveRecordInstanceMethodCall and
+        setter.asExpr().getExpr() instanceof SetterMethodCall
+      )
+    }
+
+    override DataFlow::Node getValue() { assignNode.getRhs() = result.asExpr() }
+  }
+}

ruby/ql/test/library-tests/concepts/PersistentWriteAccess.expected

@@ -0,0 +1,22 @@
+| app/controllers/users_controller.rb:5:7:5:44 | call to create! | app/controllers/users_controller.rb:5:26:5:29 | "U1" |
+| app/controllers/users_controller.rb:5:7:5:44 | call to create! | app/controllers/users_controller.rb:5:37:5:43 | call to get_uid |
+| app/controllers/users_controller.rb:6:7:6:29 | call to create | app/controllers/users_controller.rb:6:25:6:28 | "U2" |
+| app/controllers/users_controller.rb:7:7:7:31 | call to insert | app/controllers/users_controller.rb:7:26:7:29 | "U3" |
+| app/controllers/users_controller.rb:10:7:10:32 | call to update | app/controllers/users_controller.rb:10:28:10:31 | "U4" |
+| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:39:11:42 | "U5" |
+| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:53:11:56 | "U6" |
+| app/controllers/users_controller.rb:11:7:11:73 | call to update! | app/controllers/users_controller.rb:11:67:11:70 | "U7" |
+| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:31:14:34 | "U8" |
+| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:45:14:48 | "U9" |
+| app/controllers/users_controller.rb:14:7:14:66 | call to insert_all | app/controllers/users_controller.rb:14:59:14:63 | "U10" |
+| app/controllers/users_controller.rb:19:7:19:30 | call to update | app/controllers/users_controller.rb:19:25:19:29 | "U11" |
+| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:37:20:41 | "U12" |
+| app/controllers/users_controller.rb:20:7:20:57 | call to update_attributes | app/controllers/users_controller.rb:20:49:20:55 | call to get_uid |
+| app/controllers/users_controller.rb:23:7:23:42 | call to update_attribute | app/controllers/users_controller.rb:23:37:23:41 | "U13" |
+| app/controllers/users_controller.rb:26:7:26:15 | ... = ... | app/controllers/users_controller.rb:26:19:26:23 | "U14" |
+| app/models/user.rb:4:5:4:28 | call to update | app/models/user.rb:4:23:4:27 | "U15" |
+| app/models/user.rb:5:5:5:23 | call to update | app/models/user.rb:5:18:5:22 | "U16" |
+| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:35:6:39 | "U17" |
+| app/models/user.rb:6:5:6:56 | call to update_attributes | app/models/user.rb:6:51:6:54 | true |
+| app/models/user.rb:9:5:9:40 | call to update_attribute | app/models/user.rb:9:35:9:39 | "U18" |
+| app/models/user.rb:12:5:12:13 | ... = ... | app/models/user.rb:12:17:12:21 | "U19" |

ruby/ql/test/library-tests/concepts/PersistentWriteAccess.ql

@@ -0,0 +1,6 @@
+import codeql.ruby.DataFlow
+import codeql.ruby.Concepts
+
+query predicate persistentWriteAccesses(PersistentWriteAccess acc, DataFlow::Node value) {
+  value = acc.getValue()
+}

ruby/ql/test/library-tests/concepts/app/controllers/users_controller.rb

@@ -0,0 +1,34 @@
+module Users
+  class UsersController < ApplicationController
+    def create_or_modify
+      # CreateLikeCall
+      User.create!(name: "U1", uid: get_uid)
+      User.create(name: "U2")
+      User.insert({name: "U3"})
+
+      # UpdateLikeClassMethodCall
+      User.update(4, name: "U4")
+      User.update!([5, 6, 7], [{name: "U5"}, {name: "U6"}, {name: "U7"}])
+
+      # InsertAllLikeCall
+      User.insert_all([{name: "U8"}, {name: "U9"}, {name: "U10"}])
+
+      user = User.find(5)
+
+      # UpdateLikeInstanceMethodCall
+      user.update(name: "U11")
+      user.update_attributes({name: "U12", uid: get_uid})
+
+      # UpdateAttributeCall
+      user.update_attribute("name", "U13")
+
+      # AssignAttributeCall
+      user.name = "U14"
+      user.save
+    end
+
+    def get_uid
+      User.last.id + 1
+    end
+  end
+end

ruby/ql/test/library-tests/concepts/app/models/application_record.rb

@@ -0,0 +1,3 @@
+class ApplicationRecord < ActiveRecord::Base
+  self.abstract_class = true
+end

ruby/ql/test/library-tests/concepts/app/models/user.rb

@@ -0,0 +1,15 @@
+class User < ActiveRecord::Base
+  def t1
+    # UpdateLikeInstanceMethodCall
+    self.update(name: "U15")
+    update(name: "U16")
+    self.update_attributes({name: "U17", isAdmin: true})
+
+    # UpdateAttributeCall
+    self.update_attribute("name", "U18")
+
+    # AssignAttributeCall
+    self.name = "U19"
+    user.save
+  end
+end