Merge pull request github#15780 from p-/p--method-injection

Ruby: sinks for code injection via calls to `method`

Author: hmac

ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll

@@ -44,7 +44,8 @@ module Kernel {
    */
   private predicate isPublicKernelMethod(string method) {
     method in [
-        "class", "clone", "frozen?", "tap", "then", "yield_self", "send", "public_send", "__send__"
+        "class", "clone", "frozen?", "tap", "then", "yield_self", "send", "public_send", "__send__",
+        "method", "public_method", "singleton_method"
       ]
   }
 
@@ -176,6 +177,24 @@ module Kernel {
     override predicate runsArbitraryCode() { none() }
   }
 
+  /**
+   * A call to `method`, `public_method` or `singleton_method` which returns a method object.
+   * To actually execute the method, the `call` method needs to be called on the object.
+   * ```ruby
+   * m = method("exit")
+   * m.call()
+   * ```
+   */
+  class MethodCallCodeExecution extends CodeExecution::Range, KernelMethodCall {
+    MethodCallCodeExecution() {
+      this.getMethodName() = ["method", "public_method", "singleton_method"]
+    }
+
+    override DataFlow::Node getCode() { result = this.getArgument(0) }
+
+    override predicate runsArbitraryCode() { none() }
+  }
+
   private class TapSummary extends SimpleSummarizedCallable {
     TapSummary() { this = "tap" }
 

ruby/ql/src/change-notes/2024-03-01-method-code-injection-sinks.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Calls to `Object#method`, `Object#public_method` and `Object#singleton_method` with untrusted data are now recognised as sinks for code injection.

ruby/ql/test/library-tests/frameworks/core/Kernel.expected

@@ -42,6 +42,13 @@ sendCallCodeExecutions
 | Eval.rb:7:1:7:19 | call to send | Eval.rb:7:8:7:13 | "push" |
 | Kernel.rb:2:1:2:22 | call to send | Kernel.rb:2:6:2:12 | "raise" |
 | Kernel.rb:5:1:5:19 | call to send | Kernel.rb:5:8:5:13 | "push" |
+methodCallCodeExecutions
+| Kernel.rb:92:1:92:14 | call to method | Kernel.rb:92:8:92:13 | "exit" |
+| Kernel.rb:93:1:93:21 | call to public_method | Kernel.rb:93:15:93:20 | "exit" |
+| Kernel.rb:94:1:94:23 | call to singleton_method | Kernel.rb:94:18:94:22 | "foo" |
+| Kernel.rb:96:1:96:18 | call to method | Kernel.rb:96:12:96:17 | "exit" |
+| Kernel.rb:97:1:97:25 | call to public_method | Kernel.rb:97:19:97:24 | "exit" |
+| Kernel.rb:98:1:98:27 | call to singleton_method | Kernel.rb:98:22:98:26 | "foo" |
 evalCallCodeExecutions
 | Eval.rb:3:1:3:43 | call to eval | Eval.rb:3:6:3:22 | "raise \\"error\\"" |
 | Kernel.rb:1:1:1:43 | call to eval | Kernel.rb:1:6:1:22 | "raise \\"error\\"" |

ruby/ql/test/library-tests/frameworks/core/Kernel.ql

@@ -9,4 +9,6 @@ query predicate kernelSpawnCallExecutions(KernelSpawnCall c) { any() }
 
 query DataFlow::Node sendCallCodeExecutions(SendCallCodeExecution e) { result = e.getCode() }
 
+query DataFlow::Node methodCallCodeExecutions(MethodCallCodeExecution e) { result = e.getCode() }
+
 query DataFlow::Node evalCallCodeExecutions(EvalCallCodeExecution e) { result = e.getCode() }

ruby/ql/test/library-tests/frameworks/core/Kernel.rb

@@ -87,4 +87,12 @@ def run
   end
 end
 
-UnknownModule.system("ls")
+UnknownModule.system("ls")
+
+method("exit").call
+public_method("exit").call
+singleton_method("foo").call
+
+Foo.method("exit").call
+Foo.public_method("exit").call
+Foo.singleton_method("foo").call