Merge pull request #719 from owen-mc/bugfix/find-callee-through-function-variables

Look for callees through function variables

Author: owen-mc

ql/lib/change-notes/2022-04-14-fix-callee-via-variable.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The method predicate `getACalleeIncludingExternals` on `DataFlow::CallNode` and the function `viableCallable` in `DataFlowDispatch` now also work for calls to functions via a variable, where the function can be determined using local flow.

ql/lib/semmle/go/Expr.qll

@@ -784,7 +784,14 @@ class CallExpr extends CallOrConversionExpr {
   /** Gets the number of argument expressions of this call. */
   int getNumArgument() { result = count(this.getAnArgument()) }
 
-  /** Gets the name of the invoked function or method if it can be determined syntactically. */
+  /**
+   * Gets the name of the invoked function, method or variable if it can be
+   * determined syntactically.
+   *
+   * Note that if a variable is being called then this gets the variable name
+   * rather than the name of the function or method that has been assigned to
+   * the variable.
+   */
   string getCalleeName() {
     exists(Expr callee | callee = this.getCalleeExpr().stripParens() |
       result = callee.(Ident).getName()

ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll

@@ -10,7 +10,7 @@ private predicate isInterfaceCallReceiver(
 ) {
   call.getReceiver() = recv and
   recv.getType().getUnderlyingType() = tp and
-  m = call.getCalleeName()
+  m = call.getACalleeIncludingExternals().asFunction().getName()
 }
 
 /** Gets a data-flow node that may flow into the receiver value of `call`, which is an interface value. */

ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -470,6 +470,8 @@ module Public {
       exists(DataFlow::Node calleeSource | calleeSource = this.getACalleeSource() |
         result.asFuncLit() = calleeSource.asExpr()
         or
+        calleeSource = result.asFunction().getARead()
+        or
         exists(Method declared, Method actual |
           calleeSource = declared.getARead() and
           actual.implements(declared) and
@@ -484,8 +486,15 @@ module Public {
      */
     FuncDef getACallee() { result = this.getACalleeIncludingExternals().getFuncDef() }
 
-    /** Gets the name of the function or method being called, if it can be determined. */
-    string getCalleeName() { result = expr.getTarget().getName() or result = expr.getCalleeName() }
+    /**
+     * Gets the name of the function, method or variable that is being called.
+     *
+     * Note that if we are calling a variable then this gets the variable name.
+     * It does not attempt to get the name of the function or method that is
+     * assigned to the variable. To do that, use
+     * `getACalleeIncludingExternals().asFunction().getName()`.
+     */
+    string getCalleeName() { result = expr.getCalleeName() }
 
     /** Gets the data flow node specifying the function to be called. */
     Node getCalleeNode() { result = DataFlow::exprNode(expr.getCalleeExpr()) }

ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected

@@ -4,3 +4,4 @@ spuriousCallee
 | main.go:44:3:44:7 | call to m | main.go:21:1:21:20 | function declaration |
 | main.go:56:2:56:6 | call to m | main.go:21:1:21:20 | function declaration |
 | test.go:42:2:42:13 | call to Write | test.go:36:1:39:1 | function declaration |
+| test.go:44:2:44:8 | call to f1 | test.go:36:1:39:1 | function declaration |

ql/test/library-tests/semmle/go/dataflow/CallGraph/test.go

@@ -40,10 +40,20 @@ func (_ MockWriter) Write(p []byte) (n int, err error) { // name: MockWriter.Wri
 
 func test5(h hash.Hash, w io.Writer) { // name: test5
 	h.Write(nil) // callee: MockHash.Write
+	f1 := h.Write
+	f1(nil) // callee: MockHash.Write
+
 	w.Write(nil) // callee: MockWriter.Write callee: MockHash.Write
-	h.Reset()    // callee: Resetter.Reset
+	f2 := w.Write
+	f2(nil) // callee: MockWriter.Write callee: MockHash.Write
+
+	h.Reset() // callee: Resetter.Reset
+	f3 := h.Reset
+	f3() // callee: Resetter.Reset
 }
 
 func test6(h MockHash, w MockWriter) {
 	test5(h, w) // callee: test5
+	f := test5
+	f(h, w) // callee: test5
 }