Update multiple calls queries to include call targets in alert message

Author: joefarebrother

python/ql/src/Classes/CallsToInitDel/MethodCallOrder.qll

@@ -5,11 +5,14 @@ import semmle.python.ApiGraphs
 import semmle.python.dataflow.new.internal.DataFlowDispatch
 import codeql.util.Option
 
-predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, string name) {
-  exists(DataFlow::MethodCallNode call1, DataFlow::MethodCallNode call2, Class cls |
+predicate multipleCallsToSuperclassMethod(
+  Function meth, Function calledMulti, DataFlow::MethodCallNode call1,
+  DataFlow::MethodCallNode call2, string name
+) {
+  exists(Class cls |
     meth.getName() = name and
     meth.getScope() = cls and
-    call1.asExpr() != call2.asExpr() and
+    call1.getLocation().toString() < call2.getLocation().toString() and
     calledMulti = getASuperCallTargetFromCall(cls, meth, call1, name) and
     calledMulti = getASuperCallTargetFromCall(cls, meth, call2, name) and
     nonTrivial(calledMulti)
@@ -18,23 +21,44 @@ predicate multipleCallsToSuperclassMethod(Function meth, Function calledMulti, s
 
 Function getASuperCallTargetFromCall(
   Class mroBase, Function meth, DataFlow::MethodCallNode call, string name
+) {
+  exists(Function target | target = getDirectSuperCallTargetFromCall(mroBase, meth, call, name) |
+    result = target
+    or
+    result = getASuperCallTargetFromCall(mroBase, target, _, name)
+  )
+}
+
+Function getDirectSuperCallTargetFromCall(
+  Class mroBase, Function meth, DataFlow::MethodCallNode call, string name
 ) {
   meth = call.getScope() and
   getADirectSuperclass*(mroBase) = meth.getScope() and
   meth.getName() = name and
   call.calls(_, name) and
-  exists(Class targetCls | result = getASuperCallTargetFromClass(mroBase, targetCls, name) |
+  mroBase = getADirectSubclass*(meth.getScope()) and
+  exists(Class targetCls |
+    // the differences between 0-arg and 2-arg super is not considered; we assume each super uses the mro of the instance `self`
     superCall(call, _) and
-    targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase)
+    targetCls = getNextClassInMroKnownStartingClass(meth.getScope(), mroBase) and
+    result = findFunctionAccordingToMroKnownStartingClass(targetCls, mroBase, name)
     or
-    callsMethodOnClassWithSelf(meth, call, targetCls, _)
+    // targetCls is the mro base for this lookup.
+    // note however that if the call we find uses super(), that still uses the mro of the instance `self` will sill be used
+    // assuming it's 0-arg or is 2-arg with `self` as second arg.
+    callsMethodOnClassWithSelf(meth, call, targetCls, _) and
+    result = findFunctionAccordingToMroKnownStartingClass(targetCls, targetCls, name)
   )
 }
 
 Function getASuperCallTargetFromClass(Class mroBase, Class cls, string name) {
   exists(Function target |
     target = findFunctionAccordingToMroKnownStartingClass(cls, mroBase, name) and
-    (result = target or result = getASuperCallTargetFromCall(mroBase, target, _, name))
+    (
+      result = target
+      or
+      result = getASuperCallTargetFromCall(mroBase, target, _, name)
+    )
   )
 }
 

python/ql/src/Classes/CallsToInitDel/SuperclassDelCalledMultipleTimes.ql

@@ -14,17 +14,34 @@
 import python
 import MethodCallOrder
 
-predicate multipleCallsToSuperclassDel(Function meth, Function calledMulti) {
-  multipleCallsToSuperclassMethod(meth, calledMulti, "__del__")
+predicate multipleCallsToSuperclassDel(
+  Function meth, Function calledMulti, DataFlow::MethodCallNode call1,
+  DataFlow::MethodCallNode call2
+) {
+  multipleCallsToSuperclassMethod(meth, calledMulti, call1, call2, "__del__")
 }
 
-from Function meth, Function calledMulti
+from
+  Function meth, Function calledMulti, DataFlow::MethodCallNode call1,
+  DataFlow::MethodCallNode call2, Function target1, Function target2, string msg
 where
-  multipleCallsToSuperclassDel(meth, calledMulti) and
-  // Don't alert for multiple calls to a superclass del when a subclass will do.
+  multipleCallsToSuperclassDel(meth, calledMulti, call1, call2) and
+  // Only alert for the lowest method in the hierarchy that both calls will call.
   not exists(Function subMulti |
-    multipleCallsToSuperclassDel(meth, subMulti) and
+    multipleCallsToSuperclassDel(meth, subMulti, _, _) and
     calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope())
+  ) and
+  target1 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call1, _) and
+  target2 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call2, _) and
+  (
+    target1 != target2 and
+    msg =
+      "This deletion method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively."
+    or
+    target1 = target2 and
+    // The targets themselves are called multiple times (either is calledMulti, or something earlier in the MRO)
+    // Mentioning them again would be redundant.
+    msg = "This deletion method calls $@ multiple times, via $@ and $@."
   )
-select meth, "This delete method calls $@ multiple times.", calledMulti,
-  calledMulti.getQualifiedName()
+select meth, msg, calledMulti, calledMulti.getQualifiedName(), call1, "this call", call2,
+  "this call", target1, target1.getQualifiedName(), target2, target2.getQualifiedName()

python/ql/src/Classes/CallsToInitDel/SuperclassInitCalledMultipleTimes.ql

@@ -14,17 +14,34 @@
 import python
 import MethodCallOrder
 
-predicate multipleCallsToSuperclassInit(Function meth, Function calledMulti) {
-  multipleCallsToSuperclassMethod(meth, calledMulti, "__init__")
+predicate multipleCallsToSuperclassInit(
+  Function meth, Function calledMulti, DataFlow::MethodCallNode call1,
+  DataFlow::MethodCallNode call2
+) {
+  multipleCallsToSuperclassMethod(meth, calledMulti, call1, call2, "__init__")
 }
 
-from Function meth, Function calledMulti
+from
+  Function meth, Function calledMulti, DataFlow::MethodCallNode call1,
+  DataFlow::MethodCallNode call2, Function target1, Function target2, string msg
 where
-  multipleCallsToSuperclassInit(meth, calledMulti) and
-  // Don't alert for multiple calls to a superclass init when a subclass will do.
+  multipleCallsToSuperclassInit(meth, calledMulti, call1, call2) and
+  // Only alert for the lowest method in the hierarchy that both calls will call.
   not exists(Function subMulti |
-    multipleCallsToSuperclassInit(meth, subMulti) and
+    multipleCallsToSuperclassInit(meth, subMulti, _, _) and
     calledMulti.getScope() = getADirectSuperclass+(subMulti.getScope())
+  ) and
+  target1 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call1, _) and
+  target2 = getDirectSuperCallTargetFromCall(meth.getScope(), meth, call2, _) and
+  (
+    target1 != target2 and
+    msg =
+      "This initialization method calls $@ multiple times, via $@ and $@, resolving to $@ and $@ respectively."
+    or
+    target1 = target2 and
+    // The targets themselves are called multiple times (either is calledMulti, or something earlier in the MRO)
+    // Mentioning them again would be redundant.
+    msg = "This initialization method calls $@ multiple times, via $@ and $@."
   )
-select meth, "This initialization method calls $@ multiple times.", calledMulti,
-  calledMulti.getQualifiedName()
+select meth, msg, calledMulti, calledMulti.getQualifiedName(), call1, "this call", call2,
+  "this call", target1, target1.getQualifiedName(), target2, target2.getQualifiedName()