From 603c1d518efee92e66d6d94cf257acd84d58fc10 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Tue, 5 Apr 2022 16:00:14 +0100 Subject: [PATCH 1/2] Extend DataFlowCallable to include file scopes The motivation is so that getEnclosingCallable() can cope with nodes that are not in a callable. --- .../go/dataflow/internal/DataFlowDispatch.qll | 2 +- .../go/dataflow/internal/DataFlowNodes.qll | 20 +++++++---- .../go/dataflow/internal/DataFlowPrivate.qll | 36 +++++++++++++++++-- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll b/ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll index c6b1c76ddaa..7a4821cdc08 100644 --- a/ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll +++ b/ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll @@ -93,7 +93,7 @@ DataFlowCallable viableCallable(CallExpr ma) { else if isInterfaceMethodCall(call) then result = getRestrictedInterfaceTarget(call) - else result = call.getACalleeIncludingExternals() + else result.asCallable() = call.getACalleeIncludingExternals() ) } diff --git a/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll b/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll index c0fd11551a7..a855da8b557 100644 --- a/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll +++ b/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll @@ -26,11 +26,15 @@ private newtype TNode = /** Nodes intended for only use inside the data-flow libraries. */ module Private { /** Gets the callable in which this node occurs. */ - DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() } + DataFlowCallable nodeGetEnclosingCallable(Node n) { + result.asCallable() = n.getEnclosingCallable() + or + not exists(n.getEnclosingCallable()) and result.asFileScope() = n.getFile() + } /** Holds if `p` is a `ParameterNode` of `c` with position `pos`. */ predicate isParameterNode(ParameterNode p, DataFlowCallable c, int pos) { - p.isParameterOf(c, pos) + p.isParameterOf(c.asCallable(), pos) } /** A data flow node that represents returning a value from a function. */ @@ -108,9 +112,11 @@ module Public { Callable getEnclosingCallable() { result.getFuncDef() = this.getRoot() or - this = MkSummarizedParameterNode(result, _) - or - this = MkSummaryInternalNode(result, _) + exists(DataFlowCallable dfc | result = dfc.asCallable() | + this = MkSummarizedParameterNode(dfc, _) + or + this = MkSummaryInternalNode(dfc, _) + ) } /** Gets the type of this node. */ @@ -570,7 +576,9 @@ module Public { Callable c; int i; - SummarizedParameterNode() { this = MkSummarizedParameterNode(c, i) } + SummarizedParameterNode() { + this = MkSummarizedParameterNode(any(DataFlowCallable dfc | c = dfc.asCallable()), i) + } // There are no AST representations of summarized parameter nodes override ControlFlow::Root getRoot() { none() } diff --git a/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll b/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll index 16db50088de..9b8cfb194d7 100644 --- a/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll +++ b/ql/lib/semmle/go/dataflow/internal/DataFlowPrivate.qll @@ -197,7 +197,35 @@ class DataFlowType = Type; class DataFlowLocation = Location; -class DataFlowCallable = Callable; +private newtype TDataFlowCallable = + TCallable(Callable c) or + TFileScope(File f) + +class DataFlowCallable extends TDataFlowCallable { + Callable asCallable() { this = TCallable(result) } + + File asFileScope() { this = TFileScope(result) } + + FuncDef getFuncDef() { result = this.asCallable().getFuncDef() } + + Function asFunction() { result = this.asCallable().asFunction() } + + FuncLit asFuncLit() { result = this.asCallable().asFuncLit() } + + SignatureType getType() { result = this.asCallable().getType() } + + string toString() { + result = this.asCallable().toString() or + result = "File scope: " + this.asFileScope().toString() + } + + predicate hasLocationInfo( + string filepath, int startline, int startcolumn, int endline, int endcolumn + ) { + this.asCallable().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) or + this.asFileScope().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) + } +} /** A function call relevant for data flow. */ class DataFlowCall extends Expr { @@ -214,7 +242,11 @@ class DataFlowCall extends Expr { ExprNode getNode() { result = call } /** Gets the enclosing callable of this call. */ - DataFlowCallable getEnclosingCallable() { result.getFuncDef() = this.getEnclosingFunction() } + DataFlowCallable getEnclosingCallable() { + result.asCallable().getFuncDef() = this.getEnclosingFunction() + or + not exists(this.getEnclosingFunction()) and result.asFileScope() = this.getFile() + } } /** Holds if `e` is an expression that always has the same Boolean value `val`. */ From b9ff1ccd458a9c804b071d730c029b5e5bbda2c4 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan Date: Fri, 8 Apr 2022 15:23:24 +0100 Subject: [PATCH 2/2] Add change note --- ql/lib/change-notes/2022-04-08-fix-global-dataflow.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ql/lib/change-notes/2022-04-08-fix-global-dataflow.md diff --git a/ql/lib/change-notes/2022-04-08-fix-global-dataflow.md b/ql/lib/change-notes/2022-04-08-fix-global-dataflow.md new file mode 100644 index 00000000000..31e16c8b3aa --- /dev/null +++ b/ql/lib/change-notes/2022-04-08-fix-global-dataflow.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed a bug where dataflow steps were ignored if both ends were inside the initialiser routine of a file-level variable.