Merge pull request #10895 from hvitved/ruby/track-module-no-self-params

Ruby: Block for steps into `self` parameters in `trackModuleAccess`

Author: hvitved

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

@@ -537,9 +537,14 @@ private DataFlow::LocalSourceNode trackModuleAccess(Module m, TypeTracker t) {
   )
 }
 
+/**
+ * We exclude steps into `self` parameters, and instead rely on the type of the
+ * enclosing module.
+ */
 pragma[nomagic]
 private DataFlow::LocalSourceNode trackModuleAccessRec(Module m, TypeTracker t, StepSummary summary) {
-  StepSummary::step(trackModuleAccess(m, t), result, summary)
+  StepSummary::step(trackModuleAccess(m, t), result, summary) and
+  not result instanceof SelfParameterNode
 }
 
 pragma[nomagic]
@@ -603,17 +608,22 @@ private predicate isInstance(DataFlow::Node n, Module tp, boolean exact) {
   or
   exists(RelevantCall call, DataFlow::LocalSourceNode sourceNode |
     flowsToMethodCallReceiver(call, sourceNode, "new") and
-    exact = true and
     n.asExpr() = call
   |
     // `C.new`
-    sourceNode = trackModuleAccess(tp)
+    sourceNode = trackModuleAccess(tp) and
+    exact = true
     or
     // `self.new` inside a module
-    selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
+    selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp) and
+    exact = true
     or
     // `self.new` inside a singleton method
-    selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), any(SingletonMethod sm), tp)
+    exists(MethodBase target |
+      selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), target, tp) and
+      singletonMethod(target, _, _) and
+      exact = false
+    )
   )
   or
   // `self` reference in method or top-level (but not in module or singleton method,

ruby/ql/test/library-tests/modules/ancestors.expected

@@ -89,36 +89,45 @@ calls.rb:
 #  377| SingletonOverride1
 #-----| super -> Object
 
-#  404| SingletonOverride2
+#  412| SingletonOverride2
 #-----| super -> SingletonOverride1
 
-#  421| ConditionalInstanceMethods
+#  433| ConditionalInstanceMethods
 #-----| super -> Object
 
-#  484| ExtendSingletonMethod
+#  496| ExtendSingletonMethod
 
-#  494| ExtendSingletonMethod2
+#  506| ExtendSingletonMethod2
 
-#  500| ExtendSingletonMethod3
+#  512| ExtendSingletonMethod3
 
-#  513| ProtectedMethodInModule
+#  525| ProtectedMethodInModule
 
-#  519| ProtectedMethods
+#  531| ProtectedMethods
 #-----| super -> Object
 #-----| include -> ProtectedMethodInModule
 
-#  538| ProtectedMethodsSub
+#  550| ProtectedMethodsSub
 #-----| super -> ProtectedMethods
 
-#  552| SingletonUpCall_Base
+#  564| SingletonUpCall_Base
 #-----| super -> Object
 
-#  556| SingletonUpCall_Sub
+#  568| SingletonUpCall_Sub
 #-----| super -> SingletonUpCall_Base
 
-#  564| SingletonUpCall_SubSub
+#  576| SingletonUpCall_SubSub
 #-----| super -> SingletonUpCall_Sub
 
+#  583| SingletonA
+#-----| super -> Object
+
+#  596| SingletonB
+#-----| super -> SingletonA
+
+#  605| SingletonC
+#-----| super -> SingletonA
+
 hello.rb:
 #    1| EnglishWords
 

ruby/ql/test/library-tests/modules/callgraph.expected

@@ -383,6 +383,10 @@ def singleton1
         def call_singleton1
             singleton1
         end
+
+        def factory
+            self.new.instance1
+        end
     end
 
     def self.singleton2
@@ -394,6 +398,10 @@ def self.call_singleton2
     end
 
     singleton2
+
+    def instance1
+        puts "SingletonOverride1#instance1"
+    end
 end
 
 SingletonOverride1.singleton1
@@ -411,6 +419,10 @@ def singleton1
     def self.singleton2
         puts "SingletonOverride2#singleton2"
     end
+
+    def instance1
+        puts "SingletonOverride2#instance1"
+    end
 end
 
 SingletonOverride2.singleton1
@@ -567,3 +579,38 @@ def self.singleton2
 
     mid_method
 end
+
+class SingletonA
+    def self.singleton1
+    end
+
+    def self.call_singleton1
+        singleton1
+    end
+
+    def self.call_call_singleton1
+        call_singleton1
+    end
+end
+
+class SingletonB < SingletonA
+    def self.singleton1
+    end
+
+    def self.call_singleton1
+        singleton1 # should not be able to target `SingletonA:::singleton1` and `SingletonC:::singleton1`
+    end
+end
+
+class SingletonC < SingletonA
+    def self.singleton1
+    end
+
+    def self.call_singleton1
+        singleton1 # should not be able to target `SingletonA:::singleton1` and `SingletonB:::singleton1`
+    end
+end
+
+SingletonA.call_call_singleton1
+SingletonB.call_call_singleton1
+SingletonC.call_call_singleton1