Reorganise into Custimizations file + add some more sinks on ActiveRecord methods

Author: joefarebrother

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
+
+  /** A call to a method that sets attributes of an database record using a hash. */
+  private class MassAssignmentCall extends MassAssignment::Sink {
+    MassAssignmentCall() {
+      exists(DataFlow::CallNode call, string name |
+        (
+          call = activeRecordBaseClass().getAMethodCall(name)
+          or
+          call instanceof ActiveRecordInstanceMethodCall and
+          call.getMethodName() = name
+        ) and
+        (
+          name =
+            [
+              "build", "create", "create!", "create_with", "create_or_find_by",
+              "create_or_find_by!", "find_or_create_by", "find_or_create_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,55 @@
+/**
+ * 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.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 { }
+
+  private class PermitBangCall extends MassPermit instanceof DataFlow::CallNode {
+    PermitBangCall() { this.asExpr().getExpr().(MethodCall).getMethodName() = "permit!" }
+
+    override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
+
+    override DataFlow::Node getPermittedParamsResult() {
+      result = this
+      or
+      result.(DataFlow::PostUpdateNode).getPreUpdateNode() = this.getParamsArgument()
+    }
+  }
+}

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

@@ -6,49 +6,7 @@ private import codeql.ruby.AST
 private import codeql.ruby.DataFlow
 private import codeql.ruby.TaintTracking
 private import codeql.ruby.dataflow.RemoteFlowSources
-
-/** Provides default sources and sinks for the mass assignment query. */
-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 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 { }
-
-  private class CreateSink extends Sink {
-    CreateSink() {
-      exists(DataFlow::CallNode create |
-        create.asExpr().getExpr().(MethodCall).getMethodName() = ["create", "new", "update"] and
-        this = create.getArgument(0)
-      )
-    }
-  }
-
-  private class PermitCall extends MassPermit instanceof DataFlow::CallNode {
-    PermitCall() { this.asExpr().getExpr().(MethodCall).getMethodName() = "permit!" }
-
-    override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
-
-    override DataFlow::Node getPermittedParamsResult() { result = this }
-  }
-}
+private import MassAssignmentCustomizations
 
 private module FlowState {
   private newtype TState =
@@ -78,16 +36,15 @@ private module Config implements DataFlow::StateConfigSig {
   predicate isSource(DataFlow::Node node, FlowState state) {
     node instanceof MassAssignment::Source and
     state instanceof FlowState::Unpermitted
-    // or
-    // node = any(MassAssignment::MassPermit p).getPermittedParamsResult() and
-    // state instanceof FlowState::Permitted
   }
 
   predicate isSink(DataFlow::Node node, FlowState state) {
     node instanceof MassAssignment::Sink and
     state instanceof FlowState::Permitted
   }
 
+  predicate isBarrier(DataFlow::Node node) { node instanceof MassAssignment::Sanitizer }
+
   predicate isAdditionalFlowStep(
     DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
   ) {

ruby/ql/test/query-tests/security/cwe-915/MassAssignment.expected

@@ -9,4 +9,4 @@ nodes
 | test.rb:17:9:17:37 | call to permit! | semmle.label | call to permit! |
 subpaths
 #select
-| test.rb:8:18:8:28 | call to user_params | test.rb:17:9:17:14 | call to params | test.rb:8:18:8:28 | call to user_params | mass assignment |
+| test.rb:8:18:8:28 | call to user_params | test.rb:17:9:17:14 | call to params | test.rb:8:18:8:28 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:17:9:17:14 | call to params | this remote flow source |