Merge pull request github#13288 from asgerf/rb/super-and-flow-through

Ruby: two bug fixes

Author: asgerf

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

@@ -121,13 +121,15 @@ private Ruby::AstNode getSuperParent(Ruby::Super sup) {
   result = sup
   or
   result = getSuperParent(sup).getParent() and
-  not result instanceof Ruby::Method
+  not result instanceof Ruby::Method and
+  not result instanceof Ruby::SingletonMethod
 }
 
 private string getSuperMethodName(Ruby::Super sup) {
-  exists(Ruby::Method meth |
-    meth = getSuperParent(sup).getParent() and
+  exists(Ruby::AstNode meth | meth = getSuperParent(sup).getParent() |
     result = any(Method c | toGenerated(c) = meth).getName()
+    or
+    result = any(SingletonMethod c | toGenerated(c) = meth).getName()
   )
 }
 

ruby/ql/lib/codeql/ruby/typetracking/TypeTrackerSpecific.qll

@@ -89,12 +89,23 @@ private predicate flowThrough(DataFlowPublic::ParameterNode param) {
   )
 }
 
+/** Holds if there is flow from `arg` to `p` via the call `call`, not counting `new -> initialize` call steps. */
+pragma[nomagic]
+predicate callStepNoInitialize(
+  ExprNodes::CallCfgNode call, Node arg, DataFlowPrivate::ParameterNodeImpl p
+) {
+  exists(DataFlowDispatch::ParameterPosition pos |
+    argumentPositionMatch(call, arg, pos) and
+    p.isSourceParameterOf(DataFlowDispatch::getTarget(call), pos)
+  )
+}
+
 /** Holds if there is a level step from `nodeFrom` to `nodeTo`, which may depend on the call graph. */
 pragma[nomagic]
 predicate levelStepCall(Node nodeFrom, Node nodeTo) {
   exists(DataFlowPublic::ParameterNode param |
     flowThrough(param) and
-    callStep(nodeTo.asExpr(), nodeFrom, param)
+    callStepNoInitialize(nodeTo.asExpr(), nodeFrom, param)
   )
 }
 

ruby/ql/src/change-notes/2023-05-26-super-and-flow-through.md

@@ -0,0 +1,6 @@
+---
+category: minorAnalysis
+---
+* Fixed a bug that would occur when an `initialize` method returns `self` or one of its parameters.
+  In such cases, the corresponding calls to `new` would be associated with an incorrect return type.
+  This could result in inaccurate call target resolution and cause false positive alerts.