Add flow steps and sanitizers for permit calls

github · Apr 10, 2024 · 0a3d73d · 0a3d73d
1 parent 976ca48
commit 0a3d73d
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 22 deletions.
diff --git a/ruby/ql/lib/codeql/ruby/security/MassAssignmentCustomizations.qll b/ruby/ql/lib/codeql/ruby/security/MassAssignmentCustomizations.qll
@@ -41,8 +41,9 @@ module MassAssignment {
 
   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.asExpr().getExpr().(MethodCall).getMethodName() = "permit!" }
+    PermitBangCall() { this.(DataFlow::CallNode).getMethodName() = "permit!" }
 
     override DataFlow::Node getParamsArgument() { result = this.(DataFlow::CallNode).getReceiver() }
 
@@ -52,4 +53,45 @@ module MassAssignment {
       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(Expr e) {
+    e instanceof HashLiteral and
+    count(e.(HashLiteral).getAKeyValuePair()) = 0
+    or
+    hasEmptyHash(e.(HashLiteral).getAKeyValuePair().getValue())
+    or
+    hasEmptyHash(e.(Pair).getValue())
+    or
+    hasEmptyHash(e.(ArrayLiteral).getAnElement())
+  }
+
+  /** 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(_).asExpr().getExpr())
+    }
+  }
+
+  /** 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(_).asExpr().getExpr())
+    }
+
+    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 }
+  }
 }
diff --git a/ruby/ql/test/query-tests/security/cwe-915/MassAssignment.expected b/ruby/ql/test/query-tests/security/cwe-915/MassAssignment.expected
@@ -1,10 +1,4 @@
 edges
-| test.rb:23:25:23:37 | call to [] [element 0] | test.rb:23:25:23:37 | call to [] | provenance |  |
-| test.rb:23:26:23:36 | call to user_params | test.rb:23:25:23:37 | call to [] [element 0] | provenance |  |
-| test.rb:24:26:24:38 | call to [] [element 0] | test.rb:24:26:24:38 | call to [] | provenance |  |
-| test.rb:24:27:24:37 | call to user_params | test.rb:24:26:24:38 | call to [] [element 0] | provenance |  |
-| test.rb:30:21:30:33 | call to [] [element 0] | test.rb:30:21:30:33 | call to [] | provenance |  |
-| test.rb:30:22:30:32 | call to user_params | test.rb:30:21:30:33 | call to [] [element 0] | provenance |  |
 | test.rb:43:9:43:14 | call to params | test.rb:43:9:43:29 | call to require | provenance |  |
 | test.rb:43:9:43:29 | call to require | test.rb:43:9:43:37 | call to permit! | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:8:18:8:28 | call to user_params | provenance |  |
@@ -13,41 +7,37 @@ edges
 | test.rb:43:9:43:37 | call to permit! | test.rb:20:22:20:32 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:21:21:21:31 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:22:22:22:32 | call to user_params | provenance |  |
-| test.rb:43:9:43:37 | call to permit! | test.rb:23:26:23:36 | call to user_params | provenance |  |
-| test.rb:43:9:43:37 | call to permit! | test.rb:24:27:24:37 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:25:21:25:31 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:26:24:26:34 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:27:22:27:32 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:28:25:28:35 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:29:21:29:31 | call to user_params | provenance |  |
-| test.rb:43:9:43:37 | call to permit! | test.rb:30:22:30:32 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:31:32:31:42 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:32:33:32:43 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:33:36:33:46 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:34:32:34:42 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:35:33:35:43 | call to user_params | provenance |  |
 | test.rb:43:9:43:37 | call to permit! | test.rb:36:26:36:36 | call to user_params | provenance |  |
+| test.rb:47:9:47:9 | x | test.rb:48:9:48:9 | x | provenance |  |
+| test.rb:47:13:47:18 | call to params | test.rb:47:13:47:25 | ...[...] | provenance |  |
+| test.rb:47:13:47:25 | ...[...] | test.rb:47:9:47:9 | x | provenance |  |
+| test.rb:48:9:48:9 | [post] x | test.rb:49:18:49:18 | x | provenance |  |
+| test.rb:48:9:48:9 | x | test.rb:48:9:48:9 | [post] x | provenance |  |
+| test.rb:51:18:51:23 | call to params | test.rb:51:18:51:40 | call to permit | provenance |  |
+| test.rb:52:18:52:23 | call to params | test.rb:52:18:52:69 | call to permit | provenance |  |
+| test.rb:53:18:53:23 | call to params | test.rb:53:18:53:35 | call to to_unsafe_h | provenance |  |
 nodes
 | test.rb:8:18:8:28 | call to user_params | semmle.label | call to user_params |
 | test.rb:18:20:18:30 | call to user_params | semmle.label | call to user_params |
 | test.rb:19:21:19:31 | call to user_params | semmle.label | call to user_params |
 | test.rb:20:22:20:32 | call to user_params | semmle.label | call to user_params |
 | test.rb:21:21:21:31 | call to user_params | semmle.label | call to user_params |
 | test.rb:22:22:22:32 | call to user_params | semmle.label | call to user_params |
-| test.rb:23:25:23:37 | call to [] | semmle.label | call to [] |
-| test.rb:23:25:23:37 | call to [] [element 0] | semmle.label | call to [] [element 0] |
-| test.rb:23:26:23:36 | call to user_params | semmle.label | call to user_params |
-| test.rb:24:26:24:38 | call to [] | semmle.label | call to [] |
-| test.rb:24:26:24:38 | call to [] [element 0] | semmle.label | call to [] [element 0] |
-| test.rb:24:27:24:37 | call to user_params | semmle.label | call to user_params |
 | test.rb:25:21:25:31 | call to user_params | semmle.label | call to user_params |
 | test.rb:26:24:26:34 | call to user_params | semmle.label | call to user_params |
 | test.rb:27:22:27:32 | call to user_params | semmle.label | call to user_params |
 | test.rb:28:25:28:35 | call to user_params | semmle.label | call to user_params |
 | test.rb:29:21:29:31 | call to user_params | semmle.label | call to user_params |
-| test.rb:30:21:30:33 | call to [] | semmle.label | call to [] |
-| test.rb:30:21:30:33 | call to [] [element 0] | semmle.label | call to [] [element 0] |
-| test.rb:30:22:30:32 | call to user_params | semmle.label | call to user_params |
 | test.rb:31:32:31:42 | call to user_params | semmle.label | call to user_params |
 | test.rb:32:33:32:43 | call to user_params | semmle.label | call to user_params |
 | test.rb:33:36:33:46 | call to user_params | semmle.label | call to user_params |
@@ -57,6 +47,18 @@ nodes
 | test.rb:43:9:43:14 | call to params | semmle.label | call to params |
 | test.rb:43:9:43:29 | call to require | semmle.label | call to require |
 | test.rb:43:9:43:37 | call to permit! | semmle.label | call to permit! |
+| test.rb:47:9:47:9 | x | semmle.label | x |
+| test.rb:47:13:47:18 | call to params | semmle.label | call to params |
+| test.rb:47:13:47:25 | ...[...] | semmle.label | ...[...] |
+| test.rb:48:9:48:9 | [post] x | semmle.label | [post] x |
+| test.rb:48:9:48:9 | x | semmle.label | x |
+| test.rb:49:18:49:18 | x | semmle.label | x |
+| test.rb:51:18:51:23 | call to params | semmle.label | call to params |
+| test.rb:51:18:51:40 | call to permit | semmle.label | call to permit |
+| test.rb:52:18:52:23 | call to params | semmle.label | call to params |
+| test.rb:52:18:52:69 | call to permit | semmle.label | call to permit |
+| test.rb:53:18:53:23 | call to params | semmle.label | call to params |
+| test.rb:53:18:53:35 | call to to_unsafe_h | semmle.label | call to to_unsafe_h |
 subpaths
 #select
 | test.rb:8:18:8:28 | call to user_params | test.rb:43:9:43: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:43:9:43:14 | call to params | this remote flow source |
@@ -65,17 +67,18 @@ subpaths
 | test.rb:20:22:20:32 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:20:22:20:32 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:21:21:21:31 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:21:21:21:31 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:22:22:22:32 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:22:22:22:32 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
-| test.rb:23:25:23:37 | call to [] | test.rb:43:9:43:14 | call to params | test.rb:23:25:23:37 | call to [] | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
-| test.rb:24:26:24:38 | call to [] | test.rb:43:9:43:14 | call to params | test.rb:24:26:24:38 | call to [] | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:25:21:25:31 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:25:21:25:31 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:26:24:26:34 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:26:24:26:34 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:27:22:27:32 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:27:22:27:32 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:28:25:28:35 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:28:25:28:35 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:29:21:29:31 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:29:21:29:31 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
-| test.rb:30:21:30:33 | call to [] | test.rb:43:9:43:14 | call to params | test.rb:30:21:30:33 | call to [] | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:31:32:31:42 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:31:32:31:42 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:32:33:32:43 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:32:33:32:43 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:33:36:33:46 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:33:36:33:46 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:34:32:34:42 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:34:32:34:42 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:35:33:35:43 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:35:33:35:43 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
 | test.rb:36:26:36:36 | call to user_params | test.rb:43:9:43:14 | call to params | test.rb:36:26:36:36 | call to user_params | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:43:9:43:14 | call to params | this remote flow source |
+| test.rb:49:18:49:18 | x | test.rb:47:13:47:18 | call to params | test.rb:49:18:49:18 | x | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:47:13:47:18 | call to params | this remote flow source |
+| test.rb:51:18:51:40 | call to permit | test.rb:51:18:51:23 | call to params | test.rb:51:18:51:40 | call to permit | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:51:18:51:23 | call to params | this remote flow source |
+| test.rb:52:18:52:69 | call to permit | test.rb:52:18:52:23 | call to params | test.rb:52:18:52:69 | call to permit | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:52:18:52:23 | call to params | this remote flow source |
+| test.rb:53:18:53:35 | call to to_unsafe_h | test.rb:53:18:53:23 | call to params | test.rb:53:18:53:35 | call to to_unsafe_h | This mass assignment operation can assign user-controlled attributes from $@. | test.rb:53:18:53:23 | call to params | this remote flow source |
diff --git a/ruby/ql/test/query-tests/security/cwe-915/test.rb b/ruby/ql/test/query-tests/security/cwe-915/test.rb
@@ -42,4 +42,15 @@ def create3
     def user_params
         params.require(:user).permit!
     end
+
+    def create4
+        x = params[:user]
+        x.permit!
+        User.new(x) # BAD
+        User.new(x.permit(:name,:address)) # GOOD
+        User.new(params.permit(user: {})) # BAD
+        User.new(params.permit(user: [:name, :address, {friends:{}}])) # BAD
+        User.new(params.to_unsafe_h) # BAD
+        User.new(params.permit(user: [:name, :address]).to_unsafe_h) # GOOD
+     end
 end