Merge pull request #15987 from joefarebrother/ruby-mass-reassignment

Ruby: Add query for insecure mass assignment

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

@@ -792,3 +792,36 @@ class ActiveRecordScopeCallTarget extends AdditionalCallTarget {
     )
   }
 }
+
+/** Sinks for the mass assignment query. */
+private module MassAssignmentSinks {
+  private import codeql.ruby.security.MassAssignmentCustomizations
+
+  pragma[nomagic]
+  private predicate massAssignmentCall(DataFlow::CallNode call, string name) {
+    call = activeRecordBaseClass().getAMethodCall(name)
+    or
+    call instanceof ActiveRecordInstanceMethodCall and
+    call.getMethodName() = name
+  }
+
+  /** A call to a method that sets attributes of an database record using a hash. */
+  private class MassAssignmentSink extends MassAssignment::Sink {
+    MassAssignmentSink() {
+      exists(DataFlow::CallNode call, string name | massAssignmentCall(call, name) |
+        name =
+          [
+            "build", "create", "create!", "create_with", "create_or_find_by", "create_or_find_by!",
+            "find_or_create_by", "find_or_create_by!", "find_or_initialize_by", "insert", "insert!",
+            "insert_all", "insert_all!", "instantiate", "new", "update", "update!", "upsert",
+            "upsert_all"
+          ] and
+        this = call.getArgument(0)
+        or
+        // These methods have an optional first id parameter.
+        name = ["update", "update!"] and
+        this = call.getArgument(1)
+      )
+    }
+  }
+}

ruby/ql/lib/codeql/ruby/security/MassAssignmentCustomizations.qll

@@ -0,0 +1,98 @@
+/**
+ * Provides default sources, sinks, sanitizers, and flow steps for
+ * detecting insecure mass assignment, as well as extension points for adding your own.
+ */
+
+private import codeql.ruby.AST
+private import codeql.ruby.controlflow.CfgNodes
+private import codeql.ruby.DataFlow
+private import codeql.ruby.TaintTracking
+private import codeql.ruby.dataflow.RemoteFlowSources
+
+/**
+ * Provides default sources, sinks, sanitizers, and flow steps for
+ * detecting insecure mass assignment, as well as extension points for adding your own.
+ */
+module MassAssignment {
+  /**
+   * A data flow source for user input used for mass assignment.
+   */
+  abstract class Source extends DataFlow::Node { }
+
+  /**
+   * A data flow sink for user input used for mass assignment.
+   */
+  abstract class Sink extends DataFlow::Node { }
+
+  /**
+   * A sanitizer for insecure mass assignment.
+   */
+  abstract class Sanitizer extends DataFlow::Node { }
+
+  /**
+   * A call that permits arbitrary parameters to be used for mass assignment.
+   */
+  abstract class MassPermit extends DataFlow::Node {
+    /** Gets the argument for the parameters to be permitted. */
+    abstract DataFlow::Node getParamsArgument();
+
+    /** Gets the result node of the permitted parameters. */
+    abstract DataFlow::Node getPermittedParamsResult();
+  }
+
+  private class RemoteSource extends Source instanceof RemoteFlowSource { }
+
+  /** A call to `permit!`, which permits each key of its receiver. */
+  private class PermitBangCall extends MassPermit instanceof DataFlow::CallNode {
+    PermitBangCall() { this.(DataFlow::CallNode).getMethodName() = "permit!" }
+
+    override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
+
+    override DataFlow::Node getPermittedParamsResult() {
+      result = this
+      or
+      result.(DataFlow::PostUpdateNode).getPreUpdateNode() = this.getParamsArgument()
+    }
+  }
+
+  /** Holds if `h` is an empty hash or contains an empty hash at one if its (possibly nested) values. */
+  private predicate hasEmptyHash(ExprCfgNode e) {
+    e instanceof ExprNodes::HashLiteralCfgNode and
+    not exists(e.(ExprNodes::HashLiteralCfgNode).getAKeyValuePair())
+    or
+    hasEmptyHash(e.(ExprNodes::HashLiteralCfgNode).getAKeyValuePair().getValue())
+    or
+    hasEmptyHash(e.(ExprNodes::PairCfgNode).getValue())
+    or
+    hasEmptyHash(e.(ExprNodes::ArrayLiteralCfgNode).getAnArgument())
+  }
+
+  /** A call to `permit` that fully specifies the permitted parameters. */
+  private class PermitCallSanitizer extends Sanitizer, DataFlow::CallNode {
+    PermitCallSanitizer() {
+      this.getMethodName() = "permit" and
+      not hasEmptyHash(this.getArgument(_).getExprNode())
+    }
+  }
+
+  /** A call to `permit` that uses an empty hash, which allows arbitrary keys to be specified. */
+  private class PermitCallMassPermit extends MassPermit instanceof DataFlow::CallNode {
+    PermitCallMassPermit() {
+      this.(DataFlow::CallNode).getMethodName() = "permit" and
+      hasEmptyHash(this.(DataFlow::CallNode).getArgument(_).getExprNode())
+    }
+
+    override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
+
+    override DataFlow::Node getPermittedParamsResult() { result = this }
+  }
+
+  /** A call to `to_unsafe_h`, which allows arbitrary parameter. */
+  private class ToUnsafeHashCall extends MassPermit instanceof DataFlow::CallNode {
+    ToUnsafeHashCall() { this.(DataFlow::CallNode).getMethodName() = "to_unsafe_h" }
+
+    override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
+
+    override DataFlow::Node getPermittedParamsResult() { result = this }
+  }
+}

ruby/ql/lib/codeql/ruby/security/MassAssignmentQuery.qll

@@ -0,0 +1,65 @@
+/**
+ * Provides a taint tracking configuration for reasoning about insecure mass assignment.
+ */
+
+private import codeql.ruby.AST
+private import codeql.ruby.DataFlow
+private import codeql.ruby.TaintTracking
+private import codeql.ruby.dataflow.RemoteFlowSources
+private import MassAssignmentCustomizations
+
+private module FlowState {
+  private newtype TState =
+    TUnpermitted() or
+    TPermitted()
+
+  /** A flow state used to distinguish whether arbitrary user parameters have been permitted to be used for mass assignment. */
+  class State extends TState {
+    string toString() {
+      this = TUnpermitted() and result = "unpermitted"
+      or
+      this = TPermitted() and result = "permitted"
+    }
+  }
+
+  /** A flow state used for user parameters for which arbitrary parameters have not been permitted to use for mass assignment. */
+  class Unpermitted extends State, TUnpermitted { }
+
+  /** A flow state used for user parameters for which arbitrary parameters have been permitted to use for mass assignment. */
+  class Permitted extends State, TPermitted { }
+}
+
+/** A flow configuration for reasoning about insecure mass assignment. */
+private module Config implements DataFlow::StateConfigSig {
+  class FlowState = FlowState::State;
+
+  predicate isSource(DataFlow::Node node, FlowState state) {
+    node instanceof MassAssignment::Source and
+    state instanceof FlowState::Unpermitted
+  }
+
+  predicate isSink(DataFlow::Node node, FlowState state) {
+    node instanceof MassAssignment::Sink and
+    state instanceof FlowState::Permitted
+  }
+
+  predicate isBarrierIn(DataFlow::Node node, FlowState state) { isSource(node, state) }
+
+  predicate isBarrierOut(DataFlow::Node node, FlowState state) { isSink(node, state) }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof MassAssignment::Sanitizer }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
+  ) {
+    exists(MassAssignment::MassPermit permit |
+      node1 = permit.getParamsArgument() and
+      state1 instanceof FlowState::Unpermitted and
+      node2 = permit.getPermittedParamsResult() and
+      state2 instanceof FlowState::Permitted
+    )
+  }
+}
+
+/** Taint tracking for reasoning about user input used for mass assignment. */
+module MassAssignmentFlow = TaintTracking::GlobalWithState<Config>;

ruby/ql/src/change-notes/2024-03-22-mass-assignment.md

@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new query, `rb/insecure-mass-assignment`, for finding instances of mass assignment operations accepting arbitrary parameters from remote user input.

ruby/ql/src/queries/security/cwe-915/MassAssignment.qhelp

@@ -0,0 +1,34 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+	<overview>
+		<p>
+			Operations that allow for mass assignment (setting multiple attributes of an object using a hash), such as <code>ActiveRecord::Base.new</code>, should take care not to 
+            allow arbitrary parameters to be set by the user. Otherwise, unintended attributes may be set, such as an <code>is_admin</code> field for a <code>User</code> object.
+		</p>
+	</overview>
+	<recommendation>
+		<p>
+			When using a mass assignment operation from user supplied parameters, use <code>ActionController::Parameters#permit</code> to restrict the possible parameters
+            a user can supply, rather than <code>ActionController::Parameters#permit!</code>, which permits arbitrary parameters to be used for mass assignment.
+		</p>
+	</recommendation>
+	<example>
+		<p>
+			 In the following example, <code>permit!</code> is used which allows arbitrary parameters to be supplied by the user.
+		</p>
+		<sample src="examples/MassAssignmentBad.rb" />
+		<p>
+
+		</p>
+		<p>
+            In the following example, only specific parameters are permitted, so the mass assignment is safe.
+		</p>
+		<sample src="examples/MassAssignmentGood.rb" />
+	</example>
+
+	<references>
+		<li>Rails guides: <a href="https://guides.rubyonrails.org/action_controller_overview.html#strong-parameters">Strong Parameters</a>.</li>
+	</references>
+</qhelp>

ruby/ql/src/queries/security/cwe-915/MassAssignment.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Insecure Mass Assignment
+ * @description Using mass assignment with user-controlled attributes allows unintended parameters to be set.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9.8
+ * @precision high
+ * @id rb/insecure-mass-assignment
+ * @tags security
+ *       external/cwe/cwe-915
+ */
+
+import codeql.ruby.security.MassAssignmentQuery
+import MassAssignmentFlow::PathGraph
+
+from MassAssignmentFlow::PathNode source, MassAssignmentFlow::PathNode sink
+where MassAssignmentFlow::flowPath(source, sink)
+select sink.getNode(), source, sink,
+  "This mass assignment operation can assign user-controlled attributes from $@.", source.getNode(),
+  "this remote flow source"

ruby/ql/src/queries/security/cwe-915/examples/MassAssignmentBad.rb

@@ -0,0 +1,10 @@
+class UserController < ActionController::Base
+    def create
+        # BAD: arbitrary params are permitted to be used for this assignment
+        User.new(user_params).save!
+    end
+
+    def user_params
+        params.require(:user).permit!
+    end
+end

ruby/ql/src/queries/security/cwe-915/examples/MassAssignmentGood.rb

@@ -0,0 +1,10 @@
+class UserController < ActionController::Base
+    def create
+        # GOOD: the permitted parameters are explicitly specified
+        User.new(user_params).save!
+    end
+
+    def user_params
+        params.require(:user).permit(:name, :email)
+    end
+end