Ruby: flow through instance variables

2026-04-30 03:05:15 +02:00 · 2022-05-18 16:56:12 +02:00
parent c518150b49
commit d9c2b78aa2
9 changed files with 200 additions and 2 deletions
--- a/ruby/ql/lib/codeql/ruby/ast/Variable.qll
+++ b/ruby/ql/lib/codeql/ruby/ast/Variable.qll
@@ -181,6 +181,17 @@ class GlobalVariableReadAccess extends GlobalVariableAccess, VariableReadAccess
 /** An access to an instance variable. */
 class InstanceVariableAccess extends VariableAccess instanceof InstanceVariableAccessImpl {
  final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" }
+
+  /**
+   * Gets a synthetic `self` variable access.
+   */
+  final SelfVariableAccess getSelfVariableAccess() { synthChild(this, 0, result) }
+
+  final override AstNode getAChild(string pred) {
+    result = VariableAccess.super.getAChild(pred)
+    or
+    pred = "getSelfVariableAccess" and result = this.getSelfVariableAccess()
+  }
 }

 /** An access to an instance variable where the value is updated. */
--- a/ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
+++ b/ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll
@@ -210,6 +210,30 @@ private module ImplicitSelfSynthesis {
      regularMethodCallSelfSynthesis(parent, i, child)
    }
  }
+
+  /**
+   * Gets the "owner" of a node. For real nodes this is the node itself, for synthetic nodes
+   * this is the closest parent that is a real node.
+   */
+  private TAstNodeReal owner(AstNode node) { result = synthParent*(node) }
+
+  /**
+   * Gets the parent of a synthetic node
+   */
+  private AstNode synthParent(AstNode node) { node = getSynthChild(result, _) }
+
+  pragma[nomagic]
+  private predicate instanceVariableSelfSynthesis(InstanceVariableAccess var, int i, Child child) {
+    child =
+      SynthChild(SelfKind(TSelfVariable(scopeOf(toGenerated(owner(var))).getEnclosingSelfScope()))) and
+    i = 0
+  }
+
+  private class InstanceVariableSelfSynthesis extends Synthesis {
+    final override predicate child(AstNode parent, int i, Child child) {
+      instanceVariableSelfSynthesis(parent, i, child)
+    }
+  }
 }

 private module SetterDesugar {
--- a/ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll
+++ b/ruby/ql/lib/codeql/ruby/controlflow/CfgNodes.qll
@@ -615,6 +615,22 @@ module ExprNodes {
    final override VariableReadAccess getExpr() { result = ExprCfgNode.super.getExpr() }
  }

+  private class InstanceVariableAccessMapping extends ExprChildMapping, InstanceVariableAccess {
+    override predicate relevantChild(AstNode n) { n = this.getSelfVariableAccess() }
+  }
+
+  /** A control-flow node that wraps a `InstanceVariableAccess` AST expression. */
+  class InstanceVariableAccessCfgNode extends ExprCfgNode {
+    override InstanceVariableAccessMapping e;
+
+    final override InstanceVariableAccess getExpr() { result = ExprCfgNode.super.getExpr() }
+
+    /**
+     * Gets a synthetic `self` variable access.
+     */
+    final CfgNode getSelfVariableAccess() { e.hasCfgChild(e.getSelfVariableAccess(), this, result) }
+  }
+
  /** A control-flow node that wraps a `VariableWriteAccess` AST expression. */
  class VariableWriteAccessCfgNode extends ExprCfgNode {
    override VariableWriteAccess e;
--- a/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll
+++ b/ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll
@@ -1006,7 +1006,11 @@ module Trees {
    final override ControlFlowTree getChildElement(int i) { result = this.getComponent(i) }
  }

-  private class InstanceVariableTree extends LeafTree, InstanceVariableAccess { }
+  private class InstanceVariableTree extends StandardPostOrderTree, InstanceVariableAccess {
+    final override ControlFlowTree getChildElement(int i) {
+      result = this.getSelfVariableAccess() and i = 0
+    }
+  }

  private class KeywordParameterTree extends DefaultValueParameterTree, KeywordParameter {
    final override Expr getDefaultValueExpr() { result = this.getDefaultValue() }
--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
@@ -217,7 +217,10 @@ private module Cached {
    } or
    TSelfParameterNode(MethodBase m) or
    TBlockParameterNode(MethodBase m) or
-    TExprPostUpdateNode(CfgNodes::ExprCfgNode n) { n instanceof Argument } or
+    TExprPostUpdateNode(CfgNodes::ExprCfgNode n) {
+      n instanceof Argument or
+      n = any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode v).getSelfVariableAccess()
+    } or
    TSummaryNode(
      FlowSummaryImpl::Public::SummarizedCallable c,
      FlowSummaryImpl::Private::SummaryNodeState state
@@ -327,6 +330,7 @@ private module Cached {
      not cv.isInt(_) or
      cv.getInt() in [0 .. 10]
    } or
+    TFieldContent(string name) { name = any(InstanceVariable v).getName() } or
    TUnknownElementContent()

  /**
@@ -783,11 +787,42 @@ predicate jumpStep(Node pred, Node succ) {
  succ.asExpr().getExpr().(ConstantReadAccess).getValue() = pred.asExpr().getExpr()
 }

+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to
+ * content `c`.
+ */
 predicate storeStep(Node node1, ContentSet c, Node node2) {
+  // Instance variable assignment, `@var = src`
+  node2.(PostUpdateNode).getPreUpdateNode().asExpr() =
+    any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode var |
+      var.getExpr() instanceof InstanceVariableWriteAccess and
+      exists(CfgNodes::ExprNodes::AssignExprCfgNode assign |
+        var = assign.getLhs() and
+        node1.asExpr() = assign.getRhs()
+      |
+        c.isSingleton(any(Content::FieldContent ct |
+            ct.getName() = var.getExpr().getVariable().getName()
+          ))
+      )
+    ).getSelfVariableAccess()
+  or
  FlowSummaryImpl::Private::Steps::summaryStoreStep(node1, c, node2)
 }

+/**
+ * Holds if there is a read step of content `c` from `node1` to `node2`.
+ */
 predicate readStep(Node node1, ContentSet c, Node node2) {
+  // Instance variable read access, `@var`
+  node2.asExpr() =
+    any(CfgNodes::ExprNodes::InstanceVariableAccessCfgNode var |
+      var.getExpr() instanceof InstanceVariableReadAccess and
+      node1.asExpr() = var.getSelfVariableAccess() and
+      c.isSingleton(any(Content::FieldContent ct |
+          ct.getName() = var.getExpr().getVariable().getName()
+        ))
+    )
+  or
  FlowSummaryImpl::Private::Steps::summaryReadStep(node1, c, node2)
 }

--- a/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
+++ b/ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll
@@ -223,6 +223,18 @@ module Content {
    override string toString() { result = "element" }
  }

+  /** A field of an object, for example an instance variable. */
+  class FieldContent extends Content, TFieldContent {
+    private string name;
+
+    FieldContent() { this = TFieldContent(name) }
+
+    /** Gets the name of the field. */
+    string getName() { result = name }
+
+    override string toString() { result = name }
+  }
+
  /** Gets the element content corresponding to constant value `cv`. */
  ElementContent getElementContent(ConstantValue cv) {
    result = TKnownElementContent(cv)
--- a/ruby/ql/test/library-tests/dataflow/global/Flow.expected
+++ b/ruby/ql/test/library-tests/dataflow/global/Flow.expected
@@ -0,0 +1,67 @@
+failures
+edges
+| instance_variables.rb:2:19:2:19 | x :  | instance_variables.rb:3:18:3:18 | x :  |
+| instance_variables.rb:2:19:2:19 | x :  | instance_variables.rb:3:18:3:18 | x :  |
+| instance_variables.rb:3:18:3:18 | x :  | instance_variables.rb:3:9:3:14 | [post] self [@field] :  |
+| instance_variables.rb:3:18:3:18 | x :  | instance_variables.rb:3:9:3:14 | [post] self [@field] :  |
+| instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | instance_variables.rb:6:16:6:21 | self [@field] :  |
+| instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | instance_variables.rb:6:16:6:21 | self [@field] :  |
+| instance_variables.rb:6:16:6:21 | @field :  | instance_variables.rb:6:9:6:21 | return :  |
+| instance_variables.rb:6:16:6:21 | @field :  | instance_variables.rb:6:9:6:21 | return :  |
+| instance_variables.rb:6:16:6:21 | self [@field] :  | instance_variables.rb:6:16:6:21 | @field :  |
+| instance_variables.rb:6:16:6:21 | self [@field] :  | instance_variables.rb:6:16:6:21 | @field :  |
+| instance_variables.rb:11:5:11:8 | [post] self [@foo] :  | instance_variables.rb:12:10:12:13 | self [@foo] :  |
+| instance_variables.rb:11:5:11:8 | [post] self [@foo] :  | instance_variables.rb:12:10:12:13 | self [@foo] :  |
+| instance_variables.rb:11:12:11:22 | call to source :  | instance_variables.rb:11:5:11:8 | [post] self [@foo] :  |
+| instance_variables.rb:11:12:11:22 | call to source :  | instance_variables.rb:11:5:11:8 | [post] self [@foo] :  |
+| instance_variables.rb:12:10:12:13 | self [@foo] :  | instance_variables.rb:12:10:12:13 | @foo |
+| instance_variables.rb:12:10:12:13 | self [@foo] :  | instance_variables.rb:12:10:12:13 | @foo |
+| instance_variables.rb:16:1:16:3 | [post] foo [@field] :  | instance_variables.rb:17:6:17:8 | foo [@field] :  |
+| instance_variables.rb:16:1:16:3 | [post] foo [@field] :  | instance_variables.rb:17:6:17:8 | foo [@field] :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:2:19:2:19 | x :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:2:19:2:19 | x :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:16:1:16:3 | [post] foo [@field] :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:16:1:16:3 | [post] foo [@field] :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:5:5:7:7 | self in get_field [@field] :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:5:5:7:7 | self in get_field [@field] :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:17:6:17:18 | call to get_field |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:17:6:17:18 | call to get_field |
+nodes
+| instance_variables.rb:2:19:2:19 | x :  | semmle.label | x :  |
+| instance_variables.rb:2:19:2:19 | x :  | semmle.label | x :  |
+| instance_variables.rb:3:9:3:14 | [post] self [@field] :  | semmle.label | [post] self [@field] :  |
+| instance_variables.rb:3:9:3:14 | [post] self [@field] :  | semmle.label | [post] self [@field] :  |
+| instance_variables.rb:3:18:3:18 | x :  | semmle.label | x :  |
+| instance_variables.rb:3:18:3:18 | x :  | semmle.label | x :  |
+| instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | semmle.label | self in get_field [@field] :  |
+| instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | semmle.label | self in get_field [@field] :  |
+| instance_variables.rb:6:9:6:21 | return :  | semmle.label | return :  |
+| instance_variables.rb:6:9:6:21 | return :  | semmle.label | return :  |
+| instance_variables.rb:6:16:6:21 | @field :  | semmle.label | @field :  |
+| instance_variables.rb:6:16:6:21 | @field :  | semmle.label | @field :  |
+| instance_variables.rb:6:16:6:21 | self [@field] :  | semmle.label | self [@field] :  |
+| instance_variables.rb:6:16:6:21 | self [@field] :  | semmle.label | self [@field] :  |
+| instance_variables.rb:11:5:11:8 | [post] self [@foo] :  | semmle.label | [post] self [@foo] :  |
+| instance_variables.rb:11:5:11:8 | [post] self [@foo] :  | semmle.label | [post] self [@foo] :  |
+| instance_variables.rb:11:12:11:22 | call to source :  | semmle.label | call to source :  |
+| instance_variables.rb:11:12:11:22 | call to source :  | semmle.label | call to source :  |
+| instance_variables.rb:12:10:12:13 | @foo | semmle.label | @foo |
+| instance_variables.rb:12:10:12:13 | @foo | semmle.label | @foo |
+| instance_variables.rb:12:10:12:13 | self [@foo] :  | semmle.label | self [@foo] :  |
+| instance_variables.rb:12:10:12:13 | self [@foo] :  | semmle.label | self [@foo] :  |
+| instance_variables.rb:16:1:16:3 | [post] foo [@field] :  | semmle.label | [post] foo [@field] :  |
+| instance_variables.rb:16:1:16:3 | [post] foo [@field] :  | semmle.label | [post] foo [@field] :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | semmle.label | call to source :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | semmle.label | call to source :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | semmle.label | foo [@field] :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | semmle.label | foo [@field] :  |
+| instance_variables.rb:17:6:17:18 | call to get_field | semmle.label | call to get_field |
+| instance_variables.rb:17:6:17:18 | call to get_field | semmle.label | call to get_field |
+subpaths
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:2:19:2:19 | x :  | instance_variables.rb:3:9:3:14 | [post] self [@field] :  | instance_variables.rb:16:1:16:3 | [post] foo [@field] :  |
+| instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:2:19:2:19 | x :  | instance_variables.rb:3:9:3:14 | [post] self [@field] :  | instance_variables.rb:16:1:16:3 | [post] foo [@field] :  |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | instance_variables.rb:6:9:6:21 | return :  | instance_variables.rb:17:6:17:18 | call to get_field |
+| instance_variables.rb:17:6:17:8 | foo [@field] :  | instance_variables.rb:5:5:7:7 | self in get_field [@field] :  | instance_variables.rb:6:9:6:21 | return :  | instance_variables.rb:17:6:17:18 | call to get_field |
+#select
+| instance_variables.rb:12:10:12:13 | @foo | instance_variables.rb:11:12:11:22 | call to source :  | instance_variables.rb:12:10:12:13 | @foo | $@ | instance_variables.rb:11:12:11:22 | call to source :  | call to source :  |
+| instance_variables.rb:17:6:17:18 | call to get_field | instance_variables.rb:16:15:16:24 | call to source :  | instance_variables.rb:17:6:17:18 | call to get_field | $@ | instance_variables.rb:16:15:16:24 | call to source :  | call to source :  |
--- a/ruby/ql/test/library-tests/dataflow/global/Flow.ql
+++ b/ruby/ql/test/library-tests/dataflow/global/Flow.ql
@@ -0,0 +1,12 @@
+/**
+ * @kind path-problem
+ */
+
+import ruby
+import codeql.ruby.DataFlow
+private import TestUtilities.InlineFlowTest
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, DefaultValueFlowConf conf
+where conf.hasFlowPath(source, sink)
+select sink, source, sink, "$@", source, source.toString()
--- a/ruby/ql/test/library-tests/dataflow/global/instance_variables.rb
+++ b/ruby/ql/test/library-tests/dataflow/global/instance_variables.rb
@@ -0,0 +1,17 @@
+class Foo
+    def set_field x
+        @field = x
+    end
+    def get_field
+        return @field
+    end
+    def inc_field
+        @field += 1
+    end
+    @foo = source("7")
+    sink(@foo) # $ hasValueFlow=7
+
+end
+foo = Foo.new
+foo.set_field(source(42))
+sink(foo.get_field) # $ hasValueFlow=42