Distinguish interface from concrete method receivers.

Golang's historical approach has been that specifically receiver arguments are not tracked across virtual (interface) dispatch (ordinary arguments are), but models of interface methods do apply both to the receiver and ordinary arguments.

Here I approach that problem by introducing two distinct argument/parameter positions, -1 (ordinary receiver) and -2 (interface receiver). The receiver is passed in ordinary receiver position (-1) if the call is non-virtual, resulting in flow to whatever concrete method is called and perhaps a SummarizedCallable model too, while it is passed in
interface receiver position in the case of a virtual call, perhaps matching the interface receiver of a SummarizedCallable model (it cannot match a concrete method, since the interface method does not have a body).

Author: smowton

go/ql/lib/semmle/go/dataflow/ExternalFlow.qll

@@ -295,15 +295,6 @@ SourceOrSinkElement interpretElement(
   )
 }
 
-/** Holds if there is an external specification for `f`. */
-predicate hasExternalSpecification(Function f) {
-  f = any(SummarizedCallable sc).asFunction()
-  or
-  exists(SourceOrSinkElement e | f = e.asEntity() |
-    sourceElement(e, _, _, _) or sinkElement(e, _, _, _)
-  )
-}
-
 private predicate parseField(AccessPathToken c, DataFlow::FieldContent f) {
   exists(string fieldRegex, string package, string className, string fieldName |
     fieldRegex = "^Field\\[(.*)\\.([^.]+)\\.([^.]+)\\]$" and

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

@@ -117,7 +117,7 @@ predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable f) { non
 DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) { none() }
 
 private int parameterPosition() {
-  result = [-1 .. any(DataFlowCallable c).getType().getNumParameter()]
+  result = [-2 .. any(DataFlowCallable c).getType().getNumParameter()]
 }
 
 /** A parameter position represented by an integer. */

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

@@ -5,16 +5,38 @@ private import DataFlowPrivate
 private import FlowSummaryImpl as FlowSummaryImpl
 private import semmle.go.dataflow.ExternalFlow
 
+private predicate isInterfaceMethod(Method c) {
+  c.getReceiverBaseType().getUnderlyingType() instanceof InterfaceType
+}
+
+private SummaryComponent stateBottom(FlowSummaryImpl::Private::SummaryNodeState state) {
+  exists(SummaryComponentStack stack |
+    state.isInputState(_, stack) or
+    state.isOutputState(_, stack)
+  |
+    result = stack.bottom()
+  )
+}
+
 cached
 private newtype TNode =
   MkInstructionNode(IR::Instruction insn) or
   MkSsaNode(SsaDefinition ssa) or
   MkGlobalFunctionNode(Function f) or
   MkSummarizedParameterNode(SummarizedCallable c, int i) {
-    FlowSummaryImpl::Private::summaryParameterNodeRange(c, i)
+    FlowSummaryImpl::Private::summaryParameterNodeRange(c, i) and
+    if isInterfaceMethod(c.asFunction()) then i != -1 else i != -2
   } or
   MkSummaryInternalNode(SummarizedCallable c, FlowSummaryImpl::Private::SummaryNodeState state) {
-    FlowSummaryImpl::Private::summaryNodeRange(c, state)
+    FlowSummaryImpl::Private::summaryNodeRange(c, state) and
+    if isInterfaceMethod(c.asFunction())
+    then (
+      stateBottom(state) != SummaryComponent::parameter(-1) and
+      stateBottom(state) != SummaryComponent::argument(-1)
+    ) else (
+      stateBottom(state) != SummaryComponent::parameter(-2) and
+      stateBottom(state) != SummaryComponent::argument(-2)
+    )
   }
 
 /** Nodes intended for only use inside the data-flow libraries. */
@@ -34,14 +56,36 @@ module Private {
     n = MkSummaryInternalNode(result.asSummarizedCallable(), _)
   }
 
-  /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */
+  /**
+   * Holds if `p` is a `ParameterNode` of `c` with position `pos`.
+   *
+   * Note that we renumber the receiver of an interface method to -2 instead of the
+   * usual -1, in order to only propagate flow from the receiver of interface method calls
+   * to interface methods themselves (which are necessarily models), rather than to
+   * concrete methods implementing the interface, as is done for regular parameters.
+   */
   predicate isParameterNode(ParameterNode p, DataFlowCallable c, ParameterPosition pos) {
-    p.isParameterOf(c, pos)
+    exists(int realPos | p.isParameterOf(c, realPos) |
+      if realPos = -1 and isInterfaceMethod(c.asSummarizedCallable().asFunction())
+      then pos = -2
+      else realPos = pos
+    )
   }
 
-  /** Holds if `arg` is an `ArgumentNode` of `c` with position `pos`. */
+  /**
+   * Holds if `arg` is an `ArgumentNode` of `c` with position `pos`.
+   *
+   * Note that we renumber the receiver of an interface call to -2 instead of the
+   * usual -1, in order to only propagate flow from the receiver of interface method calls
+   * to interface methods themselves (which are necessarily models), rather than to
+   * concrete methods implementing the interface, as is done for regular parameters.
+   */
   predicate isArgumentNode(ArgumentNode arg, DataFlowCall c, ArgumentPosition pos) {
-    arg.argumentOf(c, pos)
+    exists(int realPos | arg.argumentOf(c, realPos) |
+      if realPos = -1 and isInterfaceMethod(c.getNode().(DataFlow::CallNode).getTarget())
+      then pos = -2
+      else realPos = pos
+    )
   }
 
   /** A data flow node that represents returning a value from a function. */
@@ -718,20 +762,8 @@ module Public {
      * Holds if this argument occurs at the given position in the given call.
      *
      * The receiver argument is considered to have index `-1`.
-     *
-     * Note that we currently do not track receiver arguments into calls to interface methods.
      */
-    predicate argumentOf(CallExpr call, int pos) {
-      call = c.asExpr() and
-      pos = i and
-      (
-        i != -1
-        or
-        exists(c.(MethodCallNode).getTarget().getBody())
-        or
-        hasExternalSpecification(c.(DataFlow::MethodCallNode).getTarget())
-      )
-    }
+    predicate argumentOf(CallExpr call, int pos) { call = c.asExpr() and pos = i }
 
     /**
      * Gets the `CallNode` this is an argument to.

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

@@ -272,10 +272,26 @@ private int parseConstantOrRange(string arg) {
   )
 }
 
+/**
+ * Gets an integer specified by the integer or range `arg`, adding `-2` in the case that the result would include `-1`.
+ */
+bindingset[arg]
+private int parseConstantOrRangeAddingInterfaceReceiver(string arg) {
+  exists(int written | written = parseConstantOrRange(arg) |
+    result = written
+    or
+    written = -1 and result = -2
+  )
+}
+
 /** Gets the argument position obtained by parsing `X` in `Parameter[X]`. */
 bindingset[arg]
-ArgumentPosition parseParamBody(string arg) { result = parseConstantOrRange(arg) }
+ArgumentPosition parseParamBody(string arg) {
+  result = parseConstantOrRangeAddingInterfaceReceiver(arg)
+}
 
 /** Gets the parameter position obtained by parsing `X` in `Argument[X]`. */
 bindingset[arg]
-ParameterPosition parseArgBody(string arg) { result = parseConstantOrRange(arg) }
+ParameterPosition parseArgBody(string arg) {
+  result = parseConstantOrRangeAddingInterfaceReceiver(arg)
+}

go/ql/test/library-tests/semmle/go/dataflow/FlowSteps/LocalTaintStep.expected

@@ -46,7 +46,7 @@
 | file://:0:0:0:0 | parameter 0 of ReadFile | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadFile |
 | file://:0:0:0:0 | parameter 0 of ReadFrom | file://:0:0:0:0 | [summary] to write: argument -1 in ReadFrom |
 | file://:0:0:0:0 | parameter 0 of ReadFrom | file://:0:0:0:0 | [summary] to write: argument -1 in ReadFrom |
-| file://:0:0:0:0 | parameter 0 of ReadFrom | file://:0:0:0:0 | [summary] to write: argument -1 in ReadFrom |
+| file://:0:0:0:0 | parameter 0 of ReadFrom | file://:0:0:0:0 | [summary] to write: argument -2 in ReadFrom |
 | file://:0:0:0:0 | parameter 0 of ReadFull | file://:0:0:0:0 | [summary] to write: argument 1 in ReadFull |
 | file://:0:0:0:0 | parameter 0 of Repeat | file://:0:0:0:0 | [summary] to write: return (return[0]) in Repeat |
 | file://:0:0:0:0 | parameter 0 of Replace | file://:0:0:0:0 | [summary] to write: return (return[0]) in Replace |
@@ -114,20 +114,20 @@
 | file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
 | file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
 | file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
-| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
-| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
-| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -1 in Write |
+| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -2 in Write |
+| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -2 in Write |
+| file://:0:0:0:0 | parameter 0 of Write | file://:0:0:0:0 | [summary] to write: argument -2 in Write |
 | file://:0:0:0:0 | parameter 0 of WriteAt | file://:0:0:0:0 | [summary] to write: argument -1 in WriteAt |
 | file://:0:0:0:0 | parameter 0 of WriteAt | file://:0:0:0:0 | [summary] to write: argument -1 in WriteAt |
-| file://:0:0:0:0 | parameter 0 of WriteAt | file://:0:0:0:0 | [summary] to write: argument -1 in WriteAt |
-| file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
+| file://:0:0:0:0 | parameter 0 of WriteAt | file://:0:0:0:0 | [summary] to write: argument -2 in WriteAt |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
 | file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -1 in WriteString |
+| file://:0:0:0:0 | parameter 0 of WriteString | file://:0:0:0:0 | [summary] to write: argument -2 in WriteString |
 | file://:0:0:0:0 | parameter 1 of AddUintptr | file://:0:0:0:0 | [summary] to write: argument 0 in AddUintptr |
 | file://:0:0:0:0 | parameter 1 of AddUintptr | file://:0:0:0:0 | [summary] to write: return (return[0]) in AddUintptr |
 | file://:0:0:0:0 | parameter 1 of AppendQuote | file://:0:0:0:0 | [summary] to write: return (return[0]) in AppendQuote |
@@ -176,15 +176,12 @@
 | file://:0:0:0:0 | parameter -1 of Get | file://:0:0:0:0 | [summary] to write: return (return[0]) in Get |
 | file://:0:0:0:0 | parameter -1 of Get | file://:0:0:0:0 | [summary] to write: return (return[0]) in Get |
 | file://:0:0:0:0 | parameter -1 of Glob | file://:0:0:0:0 | [summary] to write: return (return[0]) in Glob |
-| file://:0:0:0:0 | parameter -1 of Glob | file://:0:0:0:0 | [summary] to write: return (return[0]) in Glob |
-| file://:0:0:0:0 | parameter -1 of GoString | file://:0:0:0:0 | [summary] to write: return (return[0]) in GoString |
 | file://:0:0:0:0 | parameter -1 of GoString | file://:0:0:0:0 | [summary] to write: return (return[0]) in GoString |
 | file://:0:0:0:0 | parameter -1 of Hostname | file://:0:0:0:0 | [summary] to write: return (return[0]) in Hostname |
 | file://:0:0:0:0 | parameter -1 of Index | file://:0:0:0:0 | [summary] to write: return (return[0]) in Index |
 | file://:0:0:0:0 | parameter -1 of Info | file://:0:0:0:0 | [summary] to write: return (return[0]) in Info |
 | file://:0:0:0:0 | parameter -1 of Info | file://:0:0:0:0 | [summary] to write: return (return[0]) in Info |
 | file://:0:0:0:0 | parameter -1 of Info | file://:0:0:0:0 | [summary] to write: return (return[0]) in Info |
-| file://:0:0:0:0 | parameter -1 of Info | file://:0:0:0:0 | [summary] to write: return (return[0]) in Info |
 | file://:0:0:0:0 | parameter -1 of Interface | file://:0:0:0:0 | [summary] to write: return (return[0]) in Interface |
 | file://:0:0:0:0 | parameter -1 of InterfaceData | file://:0:0:0:0 | [summary] to write: return (return[0]) in InterfaceData |
 | file://:0:0:0:0 | parameter -1 of Key | file://:0:0:0:0 | [summary] to write: return (return[0]) in Key |
@@ -203,8 +200,6 @@
 | file://:0:0:0:0 | parameter -1 of Name | file://:0:0:0:0 | [summary] to write: return (return[0]) in Name |
 | file://:0:0:0:0 | parameter -1 of Name | file://:0:0:0:0 | [summary] to write: return (return[0]) in Name |
 | file://:0:0:0:0 | parameter -1 of Name | file://:0:0:0:0 | [summary] to write: return (return[0]) in Name |
-| file://:0:0:0:0 | parameter -1 of Name | file://:0:0:0:0 | [summary] to write: return (return[0]) in Name |
-| file://:0:0:0:0 | parameter -1 of Open | file://:0:0:0:0 | [summary] to write: return (return[0]) in Open |
 | file://:0:0:0:0 | parameter -1 of Open | file://:0:0:0:0 | [summary] to write: return (return[0]) in Open |
 | file://:0:0:0:0 | parameter -1 of Open | file://:0:0:0:0 | [summary] to write: return (return[0]) in Open |
 | file://:0:0:0:0 | parameter -1 of Parse | file://:0:0:0:0 | [summary] to write: return (return[0]) in Parse |
@@ -224,17 +219,10 @@
 | file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
 | file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
 | file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
-| file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
-| file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
-| file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
-| file://:0:0:0:0 | parameter -1 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
-| file://:0:0:0:0 | parameter -1 of ReadAt | file://:0:0:0:0 | [summary] to write: argument 0 in ReadAt |
 | file://:0:0:0:0 | parameter -1 of ReadAt | file://:0:0:0:0 | [summary] to write: argument 0 in ReadAt |
 | file://:0:0:0:0 | parameter -1 of ReadAt | file://:0:0:0:0 | [summary] to write: argument 0 in ReadAt |
 | file://:0:0:0:0 | parameter -1 of ReadAt | file://:0:0:0:0 | [summary] to write: argument 0 in ReadAt |
 | file://:0:0:0:0 | parameter -1 of ReadDir | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadDir |
-| file://:0:0:0:0 | parameter -1 of ReadDir | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadDir |
-| file://:0:0:0:0 | parameter -1 of ReadFile | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadFile |
 | file://:0:0:0:0 | parameter -1 of ReadFile | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadFile |
 | file://:0:0:0:0 | parameter -1 of Recv | file://:0:0:0:0 | [summary] to write: return (return[0]) in Recv |
 | file://:0:0:0:0 | parameter -1 of RequestURI | file://:0:0:0:0 | [summary] to write: return (return[0]) in RequestURI |
@@ -261,20 +249,12 @@
 | file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
 | file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
 | file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
-| file://:0:0:0:0 | parameter -1 of Sub | file://:0:0:0:0 | [summary] to write: return (return[0]) in Sub |
 | file://:0:0:0:0 | parameter -1 of Sub | file://:0:0:0:0 | [summary] to write: return (return[0]) in Sub |
 | file://:0:0:0:0 | parameter -1 of Swap | file://:0:0:0:0 | [summary] to write: return (return[0]) in Swap |
 | file://:0:0:0:0 | parameter -1 of Swap | file://:0:0:0:0 | [summary] to write: return (return[0]) in Swap |
 | file://:0:0:0:0 | parameter -1 of Swap | file://:0:0:0:0 | [summary] to write: return (return[0]) in Swap |
 | file://:0:0:0:0 | parameter -1 of Swap | file://:0:0:0:0 | [summary] to write: return (return[0]) in Swap |
 | file://:0:0:0:0 | parameter -1 of SyscallConn | file://:0:0:0:0 | [summary] to write: return (return[0]) in SyscallConn |
-| file://:0:0:0:0 | parameter -1 of SyscallConn | file://:0:0:0:0 | [summary] to write: return (return[0]) in SyscallConn |
-| file://:0:0:0:0 | parameter -1 of Token | file://:0:0:0:0 | [summary] to write: return (return[0]) in Token |
 | file://:0:0:0:0 | parameter -1 of Token | file://:0:0:0:0 | [summary] to write: return (return[0]) in Token |
 | file://:0:0:0:0 | parameter -1 of TryRecv | file://:0:0:0:0 | [summary] to write: return (return[0]) in TryRecv |
 | file://:0:0:0:0 | parameter -1 of UnsafeAddr | file://:0:0:0:0 | [summary] to write: return (return[0]) in UnsafeAddr |
@@ -283,7 +263,27 @@
 | file://:0:0:0:0 | parameter -1 of WriteTo | file://:0:0:0:0 | [summary] to write: argument 0 in WriteTo |
 | file://:0:0:0:0 | parameter -1 of WriteTo | file://:0:0:0:0 | [summary] to write: argument 0 in WriteTo |
 | file://:0:0:0:0 | parameter -1 of WriteTo | file://:0:0:0:0 | [summary] to write: argument 0 in WriteTo |
-| file://:0:0:0:0 | parameter -1 of WriteTo | file://:0:0:0:0 | [summary] to write: argument 0 in WriteTo |
+| file://:0:0:0:0 | parameter -2 of Glob | file://:0:0:0:0 | [summary] to write: return (return[0]) in Glob |
+| file://:0:0:0:0 | parameter -2 of GoString | file://:0:0:0:0 | [summary] to write: return (return[0]) in GoString |
+| file://:0:0:0:0 | parameter -2 of Info | file://:0:0:0:0 | [summary] to write: return (return[0]) in Info |
+| file://:0:0:0:0 | parameter -2 of Name | file://:0:0:0:0 | [summary] to write: return (return[0]) in Name |
+| file://:0:0:0:0 | parameter -2 of Open | file://:0:0:0:0 | [summary] to write: return (return[0]) in Open |
+| file://:0:0:0:0 | parameter -2 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
+| file://:0:0:0:0 | parameter -2 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
+| file://:0:0:0:0 | parameter -2 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
+| file://:0:0:0:0 | parameter -2 of Read | file://:0:0:0:0 | [summary] to write: argument 0 in Read |
+| file://:0:0:0:0 | parameter -2 of ReadAt | file://:0:0:0:0 | [summary] to write: argument 0 in ReadAt |
+| file://:0:0:0:0 | parameter -2 of ReadDir | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadDir |
+| file://:0:0:0:0 | parameter -2 of ReadFile | file://:0:0:0:0 | [summary] to write: return (return[0]) in ReadFile |
+| file://:0:0:0:0 | parameter -2 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
+| file://:0:0:0:0 | parameter -2 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
+| file://:0:0:0:0 | parameter -2 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
+| file://:0:0:0:0 | parameter -2 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
+| file://:0:0:0:0 | parameter -2 of String | file://:0:0:0:0 | [summary] to write: return (return[0]) in String |
+| file://:0:0:0:0 | parameter -2 of Sub | file://:0:0:0:0 | [summary] to write: return (return[0]) in Sub |
+| file://:0:0:0:0 | parameter -2 of SyscallConn | file://:0:0:0:0 | [summary] to write: return (return[0]) in SyscallConn |
+| file://:0:0:0:0 | parameter -2 of Token | file://:0:0:0:0 | [summary] to write: return (return[0]) in Token |
+| file://:0:0:0:0 | parameter -2 of WriteTo | file://:0:0:0:0 | [summary] to write: argument 0 in WriteTo |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[0] |
 | main.go:26:11:26:17 | type assertion | main.go:26:2:26:17 | ... := ...[1] |
 | main.go:38:13:38:13 | 1 | main.go:38:7:38:20 | slice literal |