Merge pull request #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