Data flow: Restrict ExprReturnNode to nodes from the body of the callable

Author: hvitved

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

@@ -4,7 +4,7 @@ private import internal.AST
 private import internal.TreeSitter
 
 /** A callable. */
-class Callable extends Expr, Scope, TCallable {
+class Callable extends StmtSequence, Expr, Scope, TCallable {
   /** Gets the number of parameters of this callable. */
   final int getNumberOfParameters() { result = count(this.getAParameter()) }
 

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

@@ -514,8 +514,8 @@ private module ReturnNodes {
   }
 
   /**
-   * A data-flow node that represents an expression returned by a callable,
-   * either using an explict `return` statement or as the expression of a method body.
+   * A data-flow node that represents an expression explicitly returned by
+   * a callable.
    */
   class ExplicitReturnNode extends ReturningNode, ReturningStatementNode {
     ExplicitReturnNode() {
@@ -531,11 +531,25 @@ private module ReturnNodes {
     }
   }
 
+  pragma[noinline]
+  private AstNode implicitReturn(Callable c, ExprNode n) {
+    exists(CfgNodes::ExprCfgNode en |
+      en = n.getExprNode() and
+      en.getASuccessor().(CfgNodes::AnnotatedExitNode).isNormal() and
+      n.(NodeImpl).getCfgScope() = c and
+      result = en.getExpr()
+    )
+    or
+    result = implicitReturn(c, n).getParent()
+  }
+
+  /**
+   * A data-flow node that represents an expression implicitly returned by
+   * a callable. An implicit return happens when an expression can be the
+   * last thing that is evaluated in the body of the callable.
+   */
   class ExprReturnNode extends ReturningNode, ExprNode {
-    ExprReturnNode() {
-      this.getExprNode().getASuccessor().(CfgNodes::AnnotatedExitNode).isNormal() and
-      this.(NodeImpl).getCfgScope() instanceof Callable
-    }
+    ExprReturnNode() { exists(Callable c | implicitReturn(c, this) = c.getAStmt()) }
 
     override ReturnKind getKind() { result instanceof NormalReturnKind }
   }

ruby/ql/test/library-tests/dataflow/local/Nodes.expected

@@ -8,7 +8,6 @@ ret
 | local_dataflow.rb:50:3:50:13 | next |
 | local_dataflow.rb:51:3:51:15 | break |
 | local_dataflow.rb:52:3:52:10 | "normal" |
-| local_dataflow.rb:57:9:57:9 | x |
 arg
 | local_dataflow.rb:3:8:3:10 | self | local_dataflow.rb:3:8:3:10 | call to p | -1 |
 | local_dataflow.rb:3:10:3:10 | a | local_dataflow.rb:3:8:3:10 | call to p | 0 |