get all calls to resolve to a unique predicate (within reason)

Author: erik-krogh

ql/ql/src/codeql_ql/ast/internal/Module.qll

@@ -229,7 +229,8 @@ private module Cached {
   pragma[noinline]
   private predicate resolveModuleRefHelper(TypeRef me, ContainerOrModule enclosing, string name) {
     enclosing = getEnclosingModule(me).getEnclosing*() and
-    name = [me.(ModuleExpr).getName(), me.(TypeExpr).getClassName()]
+    name = [me.(ModuleExpr).getName(), me.(TypeExpr).getClassName()] and
+    (not me instanceof ModuleExpr or not enclosing instanceof Folder_) // module expressions are not imports, so they can't resolve to a file (which is contained in a folder).
   }
 }
 

ql/ql/src/codeql_ql/ast/internal/Predicate.qll

@@ -88,7 +88,23 @@ private module Cached {
     )
   }
 
+  /**
+   *  Holds if `mc` is a `this.method()` call to a predicate defined in the same class.
+   * helps avoid spuriously resolving to predicates in super-classes.
+   */
+  private predicate resolveSelfClassCalls(MemberCall mc, PredicateOrBuiltin p) {
+    exists(Class c |
+      mc.getBase() instanceof ThisAccess and
+      c = mc.getEnclosingPredicate().getParent() and
+      p = c.getClassPredicate(mc.getMemberName()) and
+      p.getArity() = mc.getNumberOfArguments()
+    )
+  }
+
   private predicate resolveMemberCall(MemberCall mc, PredicateOrBuiltin p) {
+    resolveSelfClassCalls(mc, p)
+    or
+    not resolveSelfClassCalls(mc, _) and
     exists(Type t |
       t = mc.getBase().getType() and
       p = t.getClassPredicate(mc.getMemberName(), mc.getNumberOfArguments())
@@ -188,20 +204,20 @@ module PredConsistency {
     c > 1 and
     resolvePredicateExpr(pe, p)
   }
-  // This can happen with parameterized modules
-  /*
-   * query predicate multipleResolveCall(Call call, int c, PredicateOrBuiltin p) {
-   *    c =
-   *      strictcount(PredicateOrBuiltin p0 |
-   *        resolveCall(call, p0) and
-   *        // aliases are expected to resolve to multiple.
-   *        not exists(p0.(ClasslessPredicate).getAlias()) and
-   *        // overridden predicates may have multiple targets
-   *        not p0.(ClassPredicate).isOverride()
-   *      ) and
-   *    c > 1 and
-   *    resolveCall(call, p)
-   *  }
-   */
 
+  query predicate multipleResolveCall(Call call, int c, PredicateOrBuiltin p) {
+    c =
+      strictcount(PredicateOrBuiltin p0 |
+        resolveCall(call, p0) and
+        // aliases are expected to resolve to multiple.
+        not exists(p0.(ClasslessPredicate).getAlias()) and
+        // overridden predicates may have multiple targets
+        not p0.(ClassPredicate).isOverride() and
+        not p0 instanceof Relation // <- DB relations resolve to both a relation and a predicate.
+      ) and
+    c > 1 and
+    resolveCall(call, p) and
+    // parameterized modules are expected to resolve to multiple.
+    not exists(Predicate sig | not exists(sig.getBody()) and resolveCall(call, sig))
   }
+}

ql/ql/src/queries/diagnostics/EmptyConsistencies.ql

@@ -22,6 +22,8 @@ where
   or
   PredConsistency::noResolvePredicateExpr(node) and msg = "PredConsistency::noResolvePredicateExpr"
   or
+  PredConsistency::multipleResolveCall(node, _, _) and msg = "PredConsistency::multipleResolveCall"
+  or
   TypeConsistency::exprNoType(node) and msg = "TypeConsistency::exprNoType"
   or
   TypeConsistency::varDefNoType(node) and msg = "TypeConsistency::varDefNoType"

ql/ql/test/callgraph/MultiResolve.qll

@@ -40,3 +40,19 @@ class Super2 extends int {
 class Sub extends Super1, Super2 {
   override predicate foo() { Super1.super.foo() } // <- should resolve to Super1::foo()
 }
+
+module Foo {
+  predicate foo() { any() }
+}
+
+predicate test() {
+  Foo::foo() // <- should resolve to `foo` from the module above, and not from the `Foo.qll` file.
+}
+
+class Sub2 extends Super1, Super2 {
+  override predicate foo() { Super2.super.foo() } // <- should resolve to Super2::foo()
+
+  predicate test() {
+    this.foo() // <- should resolve to only the above `foo` predicate, but currently it resolves to that, and all the overrides [INCONSISTENCY]
+  }
+}

ql/ql/test/callgraph/callgraph.expected

@@ -18,10 +18,11 @@ getTarget
 | MultiResolve.qll:14:25:14:35 | PredicateCall | MultiResolve.qll:12:1:12:24 | ClasslessPredicate myFoo |
 | MultiResolve.qll:23:7:23:18 | PredicateCall | MultiResolve.qll:17:3:17:27 | ClasslessPredicate bar |
 | MultiResolve.qll:41:30:41:47 | MemberCall | MultiResolve.qll:31:3:31:27 | ClassPredicate foo |
+| MultiResolve.qll:49:3:49:12 | PredicateCall | MultiResolve.qll:45:3:45:27 | ClasslessPredicate foo |
+| MultiResolve.qll:53:30:53:47 | MemberCall | MultiResolve.qll:37:3:37:28 | ClassPredicate foo |
+| MultiResolve.qll:56:5:56:14 | MemberCall | MultiResolve.qll:53:12:53:49 | ClassPredicate foo |
 | Overrides.qll:8:30:8:39 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
-| Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
 | Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:14:12:14:43 | ClassPredicate bar |
-| Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
 | Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:22:12:22:44 | ClassPredicate bar |
 | Overrides.qll:28:3:28:9 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
 | Overrides.qll:29:3:29:10 | MemberCall | Overrides.qll:8:3:8:41 | ClassPredicate baz |