Merge pull request github#11575 from erik-krogh/kernelLoad

Rb: add Kernel methods as sinks to path-injection

Author: erik-krogh

ruby/ql/lib/change-notes/2022-12-07-kernel-paths.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Calls to `Kernel.load`, `Kernel.require`, `Kernel.autoload` are now modeled as sinks for path injection.

ruby/ql/lib/codeql/ruby/ast/Call.qll

@@ -97,6 +97,15 @@ class MethodCall extends Call instanceof MethodCallImpl {
    * ```
    *
    * the result is `"bar"`.
+   *
+   * Super calls call a method with the same name as the current method, so
+   * the result for a super call is the name of the current method.
+   * E.g:
+   * ```rb
+   * def foo
+   *  super # the result for this super call is "foo"
+   * end
+   * ```
    */
   final string getMethodName() { result = super.getMethodNameImpl() }
 
@@ -201,6 +210,8 @@ class YieldCall extends Call instanceof YieldCallImpl {
  */
 class SuperCall extends MethodCall instanceof SuperCallImpl {
   final override string getAPrimaryQlClass() { result = "SuperCall" }
+
+  override string toString() { result = "super call to " + this.getMethodName() }
 }
 
 /**

ruby/ql/lib/codeql/ruby/ast/internal/Call.qll

@@ -78,12 +78,9 @@ class RegularMethodCall extends MethodCallImpl, TRegularMethodCall {
   }
 
   final override string getMethodNameImpl() {
-    isRegularMethodCall(g) and
-    (
-      result = "call" and not exists(g.getMethod())
-      or
-      result = g.getMethod().(Ruby::Token).getValue()
-    )
+    result = "call" and not exists(g.getMethod())
+    or
+    result = g.getMethod().(Ruby::Token).getValue()
   }
 
   final override Expr getArgumentImpl(int n) { toGenerated(result) = g.getArguments().getChild(n) }
@@ -115,12 +112,26 @@ class ElementReferenceImpl extends MethodCallImpl, TElementReference {
 
 abstract class SuperCallImpl extends MethodCallImpl, TSuperCall { }
 
+private Ruby::AstNode getSuperParent(Ruby::Super sup) {
+  result = sup
+  or
+  result = getSuperParent(sup).getParent() and
+  not result instanceof Ruby::Method
+}
+
+private string getSuperMethodName(Ruby::Super sup) {
+  exists(Ruby::Method meth |
+    meth = getSuperParent(sup).getParent() and
+    result = any(Method c | toGenerated(c) = meth).getName()
+  )
+}
+
 class TokenSuperCall extends SuperCallImpl, TTokenSuperCall {
   private Ruby::Super g;
 
   TokenSuperCall() { this = TTokenSuperCall(g) }
 
-  final override string getMethodNameImpl() { result = g.getValue() }
+  final override string getMethodNameImpl() { result = getSuperMethodName(g) }
 
   final override Expr getReceiverImpl() { none() }
 
@@ -136,7 +147,7 @@ class RegularSuperCall extends SuperCallImpl, TRegularSuperCall {
 
   RegularSuperCall() { this = TRegularSuperCall(g) }
 
-  final override string getMethodNameImpl() { result = g.getMethod().(Ruby::Super).getValue() }
+  final override string getMethodNameImpl() { result = getSuperMethodName(g.getMethod()) }
 
   final override Expr getReceiverImpl() { none() }
 

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

@@ -24,9 +24,12 @@ module Kernel {
       this.asExpr().getExpr() instanceof UnknownMethodCall and
       (
         this.getReceiver().asExpr().getExpr() instanceof SelfVariableAccess and
-        isPrivateKernelMethod(this.getMethodName())
+        isPrivateKernelMethod(super.getMethodName())
         or
-        isPublicKernelMethod(this.getMethodName())
+        this.asExpr().getExpr() instanceof SuperCall and
+        isPrivateKernelMethod(super.getMethodName())
+        or
+        isPublicKernelMethod(super.getMethodName())
       )
     }
   }
@@ -92,14 +95,14 @@ module Kernel {
    * ```
    * Ruby documentation: https://docs.ruby-lang.org/en/3.0.0/Kernel.html#method-i-system
    */
-  class KernelSystemCall extends SystemCommandExecution::Range, KernelMethodCall {
+  class KernelSystemCall extends SystemCommandExecution::Range instanceof KernelMethodCall {
     KernelSystemCall() { this.getMethodName() = "system" }
 
-    override DataFlow::Node getAnArgument() { result = this.getArgument(_) }
+    override DataFlow::Node getAnArgument() { result = super.getArgument(_) }
 
     override predicate isShellInterpreted(DataFlow::Node arg) {
       // Kernel.system invokes a subshell if you provide a single string as argument
-      this.getNumberOfArguments() = 1 and arg = this.getAnArgument()
+      super.getNumberOfArguments() = 1 and arg = this.getAnArgument()
     }
   }
 
@@ -108,14 +111,14 @@ module Kernel {
    * `Kernel.exec` takes the same argument forms as `Kernel.system`. See `KernelSystemCall` for details.
    * Ruby documentation: https://docs.ruby-lang.org/en/3.0.0/Kernel.html#method-i-exec
    */
-  class KernelExecCall extends SystemCommandExecution::Range, KernelMethodCall {
+  class KernelExecCall extends SystemCommandExecution::Range instanceof KernelMethodCall {
     KernelExecCall() { this.getMethodName() = "exec" }
 
-    override DataFlow::Node getAnArgument() { result = this.getArgument(_) }
+    override DataFlow::Node getAnArgument() { result = super.getArgument(_) }
 
     override predicate isShellInterpreted(DataFlow::Node arg) {
       // Kernel.exec invokes a subshell if you provide a single string as argument
-      this.getNumberOfArguments() = 1 and arg = this.getAnArgument()
+      super.getNumberOfArguments() = 1 and arg = this.getAnArgument()
     }
   }
 
@@ -129,14 +132,14 @@ module Kernel {
    * spawn([env,] command... [,options]) -> pid
    * ```
    */
-  class KernelSpawnCall extends SystemCommandExecution::Range, KernelMethodCall {
+  class KernelSpawnCall extends SystemCommandExecution::Range instanceof KernelMethodCall {
     KernelSpawnCall() { this.getMethodName() = "spawn" }
 
-    override DataFlow::Node getAnArgument() { result = this.getArgument(_) }
+    override DataFlow::Node getAnArgument() { result = super.getArgument(_) }
 
     override predicate isShellInterpreted(DataFlow::Node arg) {
       // Kernel.spawn invokes a subshell if you provide a single string as argument
-      this.getNumberOfArguments() = 1 and arg = this.getAnArgument()
+      super.getNumberOfArguments() = 1 and arg = this.getAnArgument()
     }
   }
 
@@ -179,4 +182,19 @@ module Kernel {
       preservesValue = true
     }
   }
+
+  /** A call to e.g. `Kernel.load` that accesses a file. */
+  private class KernelFileAccess extends FileSystemAccess::Range instanceof KernelMethodCall {
+    KernelFileAccess() {
+      super.getMethodName() = ["load", "require", "require_relative", "autoload", "autoload?"]
+    }
+
+    override DataFlow::Node getAPathArgument() {
+      result = super.getArgument(0) and
+      super.getMethodName() = ["load", "require", "require_relative"]
+      or
+      result = super.getArgument(1) and
+      super.getMethodName() = ["autoload", "autoload?"]
+    }
+  }
 }

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

@@ -41,8 +41,8 @@ module StackTraceExposure {
   /**
    * A call to `Kernel#caller`, considered as a flow source.
    */
-  class KernelCallerCall extends Source, Kernel::KernelMethodCall {
-    KernelCallerCall() { this.getMethodName() = "caller" }
+  class KernelCallerCall extends Source instanceof Kernel::KernelMethodCall {
+    KernelCallerCall() { super.getMethodName() = "caller" }
   }
 
   /**

ruby/ql/src/queries/meta/internal/TaintMetrics.qll

@@ -1,5 +1,5 @@
+private import ruby
 private import codeql.files.FileSystem
-private import codeql.ruby.DataFlow
 private import codeql.ruby.dataflow.RemoteFlowSources
 private import codeql.ruby.security.CodeInjectionCustomizations
 private import codeql.ruby.security.CommandInjectionCustomizations
@@ -34,6 +34,12 @@ DataFlow::Node relevantTaintSink(string kind) {
     kind = "UnsafeDeserialization" and result instanceof UnsafeDeserialization::Sink
     or
     kind = "UrlRedirect" and result instanceof UrlRedirect::Sink
+  ) and
+  // the sink is not a string literal
+  not exists(Ast::StringLiteral str |
+    str = result.asExpr().getExpr() and
+    // ensure there is no interpolation, as that is not a literal
+    not str.getComponent(_) instanceof Ast::StringInterpolationComponent
   )
 }
 

ruby/ql/src/queries/security/cwe-022/PathInjection.ql

@@ -15,9 +15,8 @@
  *       external/cwe/cwe-099
  */
 
-import codeql.ruby.AST
+import ruby
 import codeql.ruby.security.PathInjectionQuery
-import codeql.ruby.DataFlow
 import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink

ruby/ql/test/library-tests/ast/Ast.expected

@@ -496,30 +496,30 @@ calls/calls.rb:
 #  279|         getReceiver: [ConstantReadAccess] X
 #  284|   getStmt: [ClassDeclaration] MyClass
 #  285|     getStmt: [Method] my_method
-#  286|       getStmt: [SuperCall] call to super
-#  287|       getStmt: [SuperCall] call to super
-#  288|       getStmt: [SuperCall] call to super
+#  286|       getStmt: [SuperCall] super call to my_method
+#  287|       getStmt: [SuperCall] super call to my_method
+#  288|       getStmt: [SuperCall] super call to my_method
 #  288|         getArgument: [StringLiteral] "blah"
 #  288|           getComponent: [StringTextComponent] blah
-#  289|       getStmt: [SuperCall] call to super
+#  289|       getStmt: [SuperCall] super call to my_method
 #  289|         getArgument: [IntegerLiteral] 1
 #  289|         getArgument: [IntegerLiteral] 2
 #  289|         getArgument: [IntegerLiteral] 3
-#  290|       getStmt: [SuperCall] call to super
+#  290|       getStmt: [SuperCall] super call to my_method
 #  290|         getBlock: [BraceBlock] { ... }
 #  290|           getParameter: [SimpleParameter] x
 #  290|             getDefiningAccess: [LocalVariableAccess] x
 #  290|           getStmt: [AddExpr] ... + ...
 #  290|             getAnOperand/getLeftOperand/getReceiver: [LocalVariableAccess] x
 #  290|             getAnOperand/getArgument/getRightOperand: [IntegerLiteral] 1
-#  291|       getStmt: [SuperCall] call to super
+#  291|       getStmt: [SuperCall] super call to my_method
 #  291|         getBlock: [DoBlock] do ... end
 #  291|           getParameter: [SimpleParameter] x
 #  291|             getDefiningAccess: [LocalVariableAccess] x
 #  291|           getStmt: [MulExpr] ... * ...
 #  291|             getAnOperand/getLeftOperand/getReceiver: [LocalVariableAccess] x
 #  291|             getAnOperand/getArgument/getRightOperand: [IntegerLiteral] 2
-#  292|       getStmt: [SuperCall] call to super
+#  292|       getStmt: [SuperCall] super call to my_method
 #  292|         getArgument: [IntegerLiteral] 4
 #  292|         getArgument: [IntegerLiteral] 5
 #  292|         getBlock: [BraceBlock] { ... }
@@ -528,7 +528,7 @@ calls/calls.rb:
 #  292|           getStmt: [AddExpr] ... + ...
 #  292|             getAnOperand/getLeftOperand/getReceiver: [LocalVariableAccess] x
 #  292|             getAnOperand/getArgument/getRightOperand: [IntegerLiteral] 100
-#  293|       getStmt: [SuperCall] call to super
+#  293|       getStmt: [SuperCall] super call to my_method
 #  293|         getArgument: [IntegerLiteral] 6
 #  293|         getArgument: [IntegerLiteral] 7
 #  293|         getBlock: [DoBlock] do ... end
@@ -545,7 +545,7 @@ calls/calls.rb:
 #  304|       getStmt: [MethodCall] call to super
 #  304|         getReceiver: [SelfVariableAccess] self
 #  305|       getStmt: [MethodCall] call to super
-#  305|         getReceiver: [SuperCall] call to super
+#  305|         getReceiver: [SuperCall] super call to another_method
 #  310|   getStmt: [MethodCall] call to call
 #  310|     getReceiver: [MethodCall] call to foo
 #  310|       getReceiver: [SelfVariableAccess] self
@@ -646,7 +646,7 @@ calls/calls.rb:
 #  328|             getComponent: [StringTextComponent] error
 #  331|   getStmt: [Method] foo
 #  331|     getParameter: [ForwardParameter] ...
-#  332|     getStmt: [SuperCall] call to super
+#  332|     getStmt: [SuperCall] super call to foo
 #  332|       getArgument: [ForwardedArguments] ...
 #  335|   getStmt: [Method] foo
 #  335|     getParameter: [SimpleParameter] a
@@ -1293,7 +1293,7 @@ modules/classes.rb:
 #   42|     getStmt: [Method] length
 #   43|       getStmt: [MulExpr] ... * ...
 #   43|         getAnOperand/getLeftOperand/getReceiver: [IntegerLiteral] 100
-#   43|         getAnOperand/getArgument/getRightOperand: [SuperCall] call to super
+#   43|         getAnOperand/getArgument/getRightOperand: [SuperCall] super call to length
 #   46|     getStmt: [Method] wibble
 #   47|       getStmt: [MethodCall] call to puts
 #   47|         getReceiver: [SelfVariableAccess] self