diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll b/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll index 0300d8bb76f..6d1f644ee98 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowDispatch.qll @@ -1,11 +1,76 @@ private import go private import DataFlowPrivate +/** + * Holds if `call` is an interface call to method `m`, meaning that its receiver `recv` has an + * interface type. + */ +private predicate isInterfaceCallReceiver(DataFlow::CallNode call, DataFlow::Node recv, string m) { + call.getReceiver() = recv and + recv.getType().getUnderlyingType() instanceof InterfaceType and + m = call.getCalleeName() +} + +/** Gets a data-flow node that may flow into the receiver value of `call`, which is an interface value. */ +private DataFlow::Node getInterfaceCallReceiverSource(DataFlow::CallNode call) { + isInterfaceCallReceiver(call, result.getASuccessor*(), _) +} + +/** Gets the type of `nd`, which must be a valid type and not an interface type. */ +private Type getConcreteType(DataFlow::Node nd) { + result = nd.getType() and + not result.getUnderlyingType() instanceof InterfaceType and + not result instanceof InvalidType +} + +/** + * Holds if all concrete (that is, non-interface) types of `nd` concrete types can be determined by + * local reasoning. + * + * `nd` is restricted to nodes that flow into the receiver value of an interface call, since that is + * all we are ultimately interested in. + */ +private predicate isConcreteValue(DataFlow::Node nd) { + nd = getInterfaceCallReceiverSource(_) and + ( + exists(getConcreteType(nd)) + or + forex(DataFlow::Node pred | pred = nd.getAPredecessor() | isConcreteValue(pred)) + ) +} + +/** + * Holds if `call` is an interface call to method `m` with receiver `recv`, where the concrete + * types of `recv` can be established by local reasoning. + */ +private predicate isConcreteInterfaceCall(DataFlow::Node call, DataFlow::Node recv, string m) { + isInterfaceCallReceiver(call, recv, m) and isConcreteValue(recv) +} + +/** + * Gets a function that might be called by `call`, where the receiver of `call` has interface type, + * but its concrete types can be determined by local reasoning. + */ +private FuncDecl getConcreteTarget(DataFlow::CallNode call) { + exists(DataFlow::Node recv, string m | + isConcreteInterfaceCall(call, recv, m) | + exists(Type concreteReceiverType, DeclaredFunction concreteTarget | + concreteReceiverType = getConcreteType(getInterfaceCallReceiverSource(call)) and + concreteTarget = concreteReceiverType.getMethod(m) and + result = concreteTarget.getFuncDecl() + ) + ) +} + /** * Gets a function that might be called by `call`. */ DataFlowCallable viableCallable(CallExpr ma) { - result = DataFlow::exprNode(ma).(DataFlow::CallNode).getACallee() + exists(DataFlow::CallNode call | call.asExpr() = ma | + if isConcreteInterfaceCall(call, _, _) + then result = getConcreteTarget(call) + else result = call.getACallee() + ) } /** diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected index 381d576a20b..d097ea413bf 100644 --- a/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/getACallee.expected @@ -1,2 +1,5 @@ missingCallee spuriousCallee +| main.go:44:3:44:7 | call to m | main.go:17:1:17:17 | function declaration | +| 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 | diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go b/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go index 1a10877b80a..3047f79e9d6 100644 --- a/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/main.go @@ -35,5 +35,23 @@ func test(x *s1, y s2, z s3, b mybool, i I) { } func test2(v interface{}) { - v.(interface{ m(int) }).m(0) // callee: s3.m + v.(interface{ m(int) }).m(0) // callee: s3.m +} + +func test3(v I) { + if v == nil { + v = s1{} + v.m() // callee: s1.m + } + v.m() // callee: s1.m callee: s2.m callee: mybool.m +} + +func test4(b bool) { + var v I + if b { + v = s1{} + } else { + v = &s2{} + } + v.m() // callee: s1.m callee: s2.m } diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.expected b/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.expected new file mode 100644 index 00000000000..381d576a20b --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.expected @@ -0,0 +1,2 @@ +missingCallee +spuriousCallee diff --git a/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.ql b/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.ql new file mode 100644 index 00000000000..132cbf36e4e --- /dev/null +++ b/ql/test/library-tests/semmle/go/dataflow/CallGraph/viableCallee.ql @@ -0,0 +1,26 @@ +import go +import semmle.go.dataflow.internal.DataFlowDispatch + +/** + * Gets a string `val` such that there is a comment on the same line as `l` + * that contains the substring `key: val`. + */ +string metadata(Locatable l, string key) { + exists(string f, int line, Comment c, string kv | + l.hasLocationInfo(f, line, _, _, _) and + c.hasLocationInfo(f, line, _, _, _) and + kv = c.getText().regexpFind("\\b(\\w+: \\S+)", _, _) and + key = kv.regexpCapture("(\\w+): (\\S+)", 1) and + result = kv.regexpCapture("(\\w+): (\\S+)", 2) + ) +} + +query predicate missingCallee(DataFlow::CallNode call, FuncDef callee) { + metadata(call.asExpr(), "callee") = metadata(callee, "name") and + not viableCallable(call.asExpr()) = callee +} + +query predicate spuriousCallee(DataFlow::CallNode call, FuncDef callee) { + viableCallable(call.asExpr()) = callee and + not metadata(call.asExpr(), "callee") = metadata(callee, "name") +}