From 3c024037013231d80261950ab143ba684265ef53 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 18 Jan 2022 23:36:14 +0000 Subject: [PATCH 1/3] Do not use getACall() when we only want direct calls In both of these locations we do not want calls through interface methods. --- ql/lib/semmle/go/security/ExternalAPIs.qll | 2 +- ql/lib/semmle/go/security/InsecureRandomnessCustomizations.qll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ql/lib/semmle/go/security/ExternalAPIs.qll b/ql/lib/semmle/go/security/ExternalAPIs.qll index 3b7e96c1064..a1172a66f8d 100644 --- a/ql/lib/semmle/go/security/ExternalAPIs.qll +++ b/ql/lib/semmle/go/security/ExternalAPIs.qll @@ -74,7 +74,7 @@ class ExternalAPIDataNode extends DataFlow::Node { // Not already modeled as a taint step not exists(DataFlow::Node next | TaintTracking::localTaintStep(this, next)) and // Not a call to a known safe external API - not call = any(SafeExternalAPIFunction f).getACall() + not call.getTarget() instanceof SafeExternalAPIFunction } /** Gets the called API `Function`. */ diff --git a/ql/lib/semmle/go/security/InsecureRandomnessCustomizations.qll b/ql/lib/semmle/go/security/InsecureRandomnessCustomizations.qll index c894afb4123..2cbb350461b 100644 --- a/ql/lib/semmle/go/security/InsecureRandomnessCustomizations.qll +++ b/ql/lib/semmle/go/security/InsecureRandomnessCustomizations.qll @@ -60,7 +60,7 @@ module InsecureRandomness { // Some interfaces in the `crypto` package are the same as interfaces // elsewhere, e.g. tls.listener is the same as net.Listener not fn.hasQualifiedName(nonCryptoInterface(), _) and - this = fn.getACall().getAnArgument() + exists(DataFlow::CallNode call | call.getTarget() = fn and this = call.getAnArgument()) ) } From 84f9b74f502d1667ad035c917122cf09ca92489c Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 18 Jan 2022 23:40:39 +0000 Subject: [PATCH 2/3] t Improve documentation of `Function.getACall` --- ql/lib/semmle/go/Scopes.qll | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ql/lib/semmle/go/Scopes.qll b/ql/lib/semmle/go/Scopes.qll index 91ac579b491..e217c2a28ef 100644 --- a/ql/lib/semmle/go/Scopes.qll +++ b/ql/lib/semmle/go/Scopes.qll @@ -366,11 +366,18 @@ class PromotedField extends Field { /** A built-in or declared function. */ class Function extends ValueEntity, @functionobject { - /** Gets a call to this function. */ + /** + * Gets a call to this function. + * + * This includes calls that target this function indirectly, by calling an + * interface method that this function implements. + */ pragma[nomagic] DataFlow::CallNode getACall() { + // Direct calls to this function this = result.getTarget() or + // Direct calls and calls to interface methods that this function implements this = result.getACalleeIncludingExternals().asFunction() } From 4d1dcb32604691642b0ff897309d137613ab4d7d Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Wed, 19 Jan 2022 16:00:55 +0000 Subject: [PATCH 3/3] Remove first disjunct as it is a subset of second disjunct --- ql/lib/semmle/go/Scopes.qll | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ql/lib/semmle/go/Scopes.qll b/ql/lib/semmle/go/Scopes.qll index e217c2a28ef..02ead4b9468 100644 --- a/ql/lib/semmle/go/Scopes.qll +++ b/ql/lib/semmle/go/Scopes.qll @@ -373,13 +373,7 @@ class Function extends ValueEntity, @functionobject { * interface method that this function implements. */ pragma[nomagic] - DataFlow::CallNode getACall() { - // Direct calls to this function - this = result.getTarget() - or - // Direct calls and calls to interface methods that this function implements - this = result.getACalleeIncludingExternals().asFunction() - } + DataFlow::CallNode getACall() { this = result.getACalleeIncludingExternals().asFunction() } /** Gets the declaration of this function, if any. */ FuncDecl getFuncDecl() { none() }