Merge pull request #10620 from hvitved/ruby/call-graph-protected-methods

Ruby: Account for `protected` methods in call graph

Author: hvitved

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

@@ -317,15 +317,20 @@ private module Cached {
         exists(Module tp |
           instanceMethodCall(call, tp, method) and
           result = lookupMethod(tp, method) and
-          if result.(Method).isPrivate()
-          then
-            call.getReceiver().getExpr() instanceof SelfVariableAccess and
-            // For now, we restrict the scope of top-level declarations to their file.
-            // This may remove some plausible targets, but also removes a lot of
-            // implausible targets
-            if result.getEnclosingModule() instanceof Toplevel
-            then result.getFile() = call.getFile()
+          (
+            if result.(Method).isPrivate()
+            then
+              call.getReceiver().getExpr() instanceof SelfVariableAccess and
+              // For now, we restrict the scope of top-level declarations to their file.
+              // This may remove some plausible targets, but also removes a lot of
+              // implausible targets
+              if result.getEnclosingModule() instanceof Toplevel
+              then result.getFile() = call.getFile()
+              else any()
             else any()
+          ) and
+          if result.(Method).isProtected()
+          then result = lookupMethod(call.getExpr().getEnclosingModule().getModule(), method)
           else any()
         )
         or

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

@@ -49,53 +49,62 @@ calls.rb:
 #  105| Module
 #-----| super -> Object
 
-#  112| Object
+#  115| Object
 #-----| super -> BasicObject
 #-----| include -> Kernel
 #-----| prepend -> A
 
-#  117| Hash
+#  120| Hash
 #-----| super -> Object
 
-#  122| Array
+#  125| Array
 #-----| super -> Object
 
-#  162| S
+#  165| S
 #-----| super -> Object
 
-#  168| A
+#  171| A
 #-----| super -> S
 #-----| super -> B
 #-----| prepend -> A::B
 
-#  173| B
+#  176| B
 #-----| super -> S
 
-#  187| Singletons
+#  190| Singletons
 #-----| super -> Object
 
-#  307| SelfNew
+#  310| SelfNew
 #-----| super -> Object
 
-#  322| C1
+#  325| C1
 #-----| super -> Object
 
-#  328| C2
+#  331| C2
 #-----| super -> C1
 
-#  334| C3
+#  337| C3
 #-----| super -> C2
 
-#  374| SingletonOverride1
+#  377| SingletonOverride1
 #-----| super -> Object
 
-#  399| SingletonOverride2
+#  402| SingletonOverride2
 #-----| super -> SingletonOverride1
 
-#  414| ConditionalInstanceMethods
+#  417| ConditionalInstanceMethods
 #-----| super -> Object
 
-#  477| ExtendSingletonMethod
+#  480| ExtendSingletonMethod
+
+#  490| ProtectedMethodInModule
+
+#  496| ProtectedMethods
+#-----| super -> Object
+#-----| include -> ProtectedMethodInModule
+
+#  515| ProtectedMethodsSub
+#-----| super -> ProtectedMethods
 
 hello.rb:
 #    1| EnglishWords

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

@@ -103,8 +103,11 @@ def puts x; old_puts x end
 end
 
 class Module
+    alias :old_include :include
     def module_eval; end
-    def include; end
+    def include x
+        old_include x
+    end
     def prepend; end
     def private; end
 end
@@ -447,7 +450,7 @@ def m5
 ConditionalInstanceMethods.new.m3 # currently unable to resolve
 ConditionalInstanceMethods.new.m4 # currently unable to resolve
 ConditionalInstanceMethods.new.m5 # NoMethodError
-exit
+
 EsotericInstanceMethods = Class.new do
     [0,1,2].each do
         def foo
@@ -483,3 +486,39 @@ def singleton
 end
 
 ExtendSingletonMethod.singleton # currently unable to resolve
+
+module ProtectedMethodInModule
+    protected def foo
+        puts "ProtectedMethodInModule#foo"
+    end
+end
+
+class ProtectedMethods
+    include ProtectedMethodInModule
+
+    protected def bar
+        puts "ProtectedMethods#bar"
+    end
+
+    def baz
+        foo
+        bar
+        ProtectedMethods.new.foo
+        ProtectedMethods.new.bar
+    end
+end
+
+ProtectedMethods.new.foo # NoMethodError
+ProtectedMethods.new.bar # NoMethodError
+ProtectedMethods.new.baz
+
+class ProtectedMethodsSub < ProtectedMethods
+    def baz
+        foo
+        ProtectedMethodsSub.new.foo
+    end
+end
+
+ProtectedMethodsSub.new.foo # NoMethodError
+ProtectedMethodsSub.new.bar # NoMethodError
+ProtectedMethodsSub.new.baz