Ruby: Include all assignments in data flow paths

Author: hvitved

ruby/ql/lib/codeql/ruby/dataflow/SSA.qll

@@ -216,9 +216,9 @@ module Ssa {
       )
     }
 
-    final override string toString() { result = Definition.super.toString() }
+    final override string toString() { result = write.toString() }
 
-    final override Location getLocation() { result = this.getControlFlowNode().getLocation() }
+    final override Location getLocation() { result = write.getLocation() }
   }
 
   /**

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

@@ -528,10 +528,7 @@ import Cached
 /** Holds if `n` should be hidden from path explanations. */
 predicate nodeIsHidden(Node n) {
   exists(SsaImpl::DefinitionExt def | def = n.(SsaDefinitionExtNode).getDefinitionExt() |
-    def instanceof Ssa::PhiNode or
-    def instanceof SsaImpl::PhiReadNode or
-    def instanceof Ssa::CapturedEntryDefinition or
-    def instanceof Ssa::CapturedCallDefinition
+    not def instanceof Ssa::WriteDefinition
   )
   or
   n = LocalFlow::getParameterDefNode(_)
@@ -1333,7 +1330,15 @@ private module PostUpdateNodes {
 private import PostUpdateNodes
 
 /** A node that performs a type cast. */
-class CastNode extends Node instanceof ReturningNode { }
+class CastNode extends Node {
+  CastNode() {
+    // ensure that actual return nodes are included in the path graph
+    this instanceof ReturningNode
+    or
+    // ensure that all variable assignments are included in the path graph
+    this.(SsaDefinitionExtNode).getDefinitionExt() instanceof Ssa::WriteDefinition
+  }
+}
 
 class DataFlowExpr = CfgNodes::ExprCfgNode;
 

ruby/ql/test/library-tests/dataflow/array-flow/array-flow.expected

@@ -207,8 +207,10 @@ edges
 | instance_variables.rb:114:6:114:10 | foo13 [@field] :  | instance_variables.rb:13:5:15:7 | self in get_field [@field] :  |
 | instance_variables.rb:114:6:114:10 | foo13 [@field] :  | instance_variables.rb:114:6:114:20 | call to get_field |
 | instance_variables.rb:114:6:114:10 | foo13 [@field] :  | instance_variables.rb:114:6:114:20 | call to get_field |
-| instance_variables.rb:116:9:116:26 | call to new [@field] :  | instance_variables.rb:117:6:117:10 | foo15 [@field] :  |
-| instance_variables.rb:116:9:116:26 | call to new [@field] :  | instance_variables.rb:117:6:117:10 | foo15 [@field] :  |
+| instance_variables.rb:116:1:116:5 | foo15 [@field] :  | instance_variables.rb:117:6:117:10 | foo15 [@field] :  |
+| instance_variables.rb:116:1:116:5 | foo15 [@field] :  | instance_variables.rb:117:6:117:10 | foo15 [@field] :  |
+| instance_variables.rb:116:9:116:26 | call to new [@field] :  | instance_variables.rb:116:1:116:5 | foo15 [@field] :  |
+| instance_variables.rb:116:9:116:26 | call to new [@field] :  | instance_variables.rb:116:1:116:5 | foo15 [@field] :  |
 | instance_variables.rb:116:17:116:25 | call to taint :  | instance_variables.rb:22:20:22:24 | field :  |
 | instance_variables.rb:116:17:116:25 | call to taint :  | instance_variables.rb:22:20:22:24 | field :  |
 | instance_variables.rb:116:17:116:25 | call to taint :  | instance_variables.rb:116:9:116:26 | call to new [@field] :  |
@@ -227,8 +229,10 @@ edges
 | instance_variables.rb:120:6:120:10 | foo16 [@field] :  | instance_variables.rb:13:5:15:7 | self in get_field [@field] :  |
 | instance_variables.rb:120:6:120:10 | foo16 [@field] :  | instance_variables.rb:120:6:120:20 | call to get_field |
 | instance_variables.rb:120:6:120:10 | foo16 [@field] :  | instance_variables.rb:120:6:120:20 | call to get_field |
-| instance_variables.rb:121:7:121:24 | call to new :  | instance_variables.rb:122:6:122:8 | bar |
-| instance_variables.rb:121:7:121:24 | call to new :  | instance_variables.rb:122:6:122:8 | bar |
+| instance_variables.rb:121:1:121:3 | bar :  | instance_variables.rb:122:6:122:8 | bar |
+| instance_variables.rb:121:1:121:3 | bar :  | instance_variables.rb:122:6:122:8 | bar |
+| instance_variables.rb:121:7:121:24 | call to new :  | instance_variables.rb:121:1:121:3 | bar :  |
+| instance_variables.rb:121:7:121:24 | call to new :  | instance_variables.rb:121:1:121:3 | bar :  |
 nodes
 | captured_variables.rb:1:24:1:24 | x :  | semmle.label | x :  |
 | captured_variables.rb:1:24:1:24 | x :  | semmle.label | x :  |
@@ -424,6 +428,8 @@ nodes
 | instance_variables.rb:114:6:114:10 | foo13 [@field] :  | semmle.label | foo13 [@field] :  |
 | instance_variables.rb:114:6:114:20 | call to get_field | semmle.label | call to get_field |
 | instance_variables.rb:114:6:114:20 | call to get_field | semmle.label | call to get_field |
+| instance_variables.rb:116:1:116:5 | foo15 [@field] :  | semmle.label | foo15 [@field] :  |
+| instance_variables.rb:116:1:116:5 | foo15 [@field] :  | semmle.label | foo15 [@field] :  |
 | instance_variables.rb:116:9:116:26 | call to new [@field] :  | semmle.label | call to new [@field] :  |
 | instance_variables.rb:116:9:116:26 | call to new [@field] :  | semmle.label | call to new [@field] :  |
 | instance_variables.rb:116:17:116:25 | call to taint :  | semmle.label | call to taint :  |
@@ -442,6 +448,8 @@ nodes
 | instance_variables.rb:120:6:120:10 | foo16 [@field] :  | semmle.label | foo16 [@field] :  |
 | instance_variables.rb:120:6:120:20 | call to get_field | semmle.label | call to get_field |
 | instance_variables.rb:120:6:120:20 | call to get_field | semmle.label | call to get_field |
+| instance_variables.rb:121:1:121:3 | bar :  | semmle.label | bar :  |
+| instance_variables.rb:121:1:121:3 | bar :  | semmle.label | bar :  |
 | instance_variables.rb:121:7:121:24 | call to new :  | semmle.label | call to new :  |
 | instance_variables.rb:121:7:121:24 | call to new :  | semmle.label | call to new :  |
 | instance_variables.rb:122:6:122:8 | bar | semmle.label | bar |