From 5275168253619bedf83306f7f6e8276b11b7354e Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 29 Jun 2020 10:02:16 +0100 Subject: [PATCH 1/3] Make target branch configurable for `sync-dataflow-libraries`. You can now do `make DATAFLOW_BRANCH= sync-dataflow-libraries`; default is still `master`. --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 694b32f8b31..37790e81a29 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,8 @@ clean: AUTOFORMAT=-qq -i +DATAFLOW_BRANCH=master + autoformat: find ql/src -name *.ql -or -name *.qll | xargs codeql query format $(AUTOFORMAT) @@ -119,5 +121,5 @@ build/testdb/go.dbscheme: upgrades/initial/go.dbscheme sync-dataflow-libraries: for f in DataFlowImpl.qll DataFlowImplCommon.qll tainttracking1/TaintTrackingImpl.qll;\ do\ - curl -s -o ./ql/src/semmle/go/dataflow/internal/$$f https://raw.githubusercontent.com/github/codeql/master/java/ql/src/semmle/code/java/dataflow/internal/$$f;\ + curl -s -o ./ql/src/semmle/go/dataflow/internal/$$f https://raw.githubusercontent.com/github/codeql/$(DATAFLOW_BRANCH)/java/ql/src/semmle/code/java/dataflow/internal/$$f;\ done From f7ed65692fa6b742efaea285ceb63f3827514bb8 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 29 Jun 2020 11:02:35 +0100 Subject: [PATCH 2/3] Data flow: Use `accessPathLimit()` in partial flow as well. cf. https://github.com/github/codeql/pull/3494 --- ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll index 9a7dc9dc1e3..1aeedf717f7 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll @@ -2564,7 +2564,7 @@ private module FlowExploration { private newtype TPartialAccessPath = TPartialNil(DataFlowType t) or - TPartialCons(TypedContent tc, int len) { len in [1 .. 5] } + TPartialCons(TypedContent tc, int len) { len in [1 .. accessPathLimit()] } /** * Conceptually a list of `TypedContent`s followed by a `Type`, but only the first From 2b3e3bda8f7804987c869ee0dafe57360977e3ad Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 29 Jun 2020 11:06:35 +0100 Subject: [PATCH 3/3] Data flow: Model field clearing. cf https://github.com/github/codeql/pull/3762 --- .../go/dataflow/internal/DataFlowImpl.qll | 24 ++++++++++++++----- .../dataflow/internal/DataFlowImplCommon.qll | 7 ++++++ .../go/dataflow/internal/DataFlowPrivate.qll | 7 ++++++ .../tainttracking1/TaintTrackingImpl.qll | 4 ++-- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll index 1aeedf717f7..a5c032f2660 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowImpl.qll @@ -19,7 +19,7 @@ import DataFlowImplSpecific::Public * a subclass whose characteristic predicate is a unique singleton string. * For example, write * - * ``` + * ```ql * class MyAnalysisConfiguration extends DataFlow::Configuration { * MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" } * // Override `isSource` and `isSink`. @@ -37,7 +37,7 @@ import DataFlowImplSpecific::Public * Then, to query whether there is flow between some `source` and `sink`, * write * - * ``` + * ```ql * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) * ``` * @@ -1051,6 +1051,17 @@ private predicate flowIntoCallNodeCand2( } private module LocalFlowBigStep { + /** + * A node where some checking is required, and hence the big-step relation + * is not allowed to step over. + */ + private class FlowCheckNode extends Node { + FlowCheckNode() { + this instanceof CastNode or + clearsContent(this, _) + } + } + /** * Holds if `node` can be the first node in a maximal subsequence of local * flow steps in a dataflow path. @@ -1065,7 +1076,7 @@ private module LocalFlowBigStep { node instanceof OutNodeExt or store(_, _, node, _) or read(_, _, node) or - node instanceof CastNode + node instanceof FlowCheckNode ) } @@ -1083,7 +1094,7 @@ private module LocalFlowBigStep { read(node, _, next) ) or - node instanceof CastNode + node instanceof FlowCheckNode or config.isSink(node) } @@ -1127,14 +1138,14 @@ private module LocalFlowBigStep { exists(Node mid | localFlowStepPlus(node1, mid, preservesValue, t, config, cc) and localFlowStepNodeCand1(mid, node2, config) and - not mid instanceof CastNode and + not mid instanceof FlowCheckNode and nodeCand2(node2, unbind(config)) ) or exists(Node mid | localFlowStepPlus(node1, mid, _, _, config, cc) and additionalLocalFlowStepNodeCand2(mid, node2, config) and - not mid instanceof CastNode and + not mid instanceof FlowCheckNode and preservesValue = false and t = getErasedNodeTypeBound(node2) and nodeCand2(node2, unbind(config)) @@ -1190,6 +1201,7 @@ private predicate flowCandFwd( Configuration config ) { flowCandFwd0(node, fromArg, argApf, apf, config) and + not apf.isClearedAt(node) and if node instanceof CastingNode then compatibleTypes(getErasedNodeTypeBound(node), apf.getType()) else any() diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll b/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll index 1f42c21d5a7..091ccff09d1 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowImplCommon.qll @@ -754,6 +754,13 @@ abstract class AccessPathFront extends TAccessPathFront { abstract boolean toBoolNonEmpty(); predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) } + + predicate isClearedAt(Node n) { + exists(TypedContent tc | + this.headUsesContent(tc) and + clearsContent(n, tc.getContent()) + ) + } } class AccessPathFrontNil extends AccessPathFront, TFrontNil { diff --git a/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll b/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll index b60f1381d84..1d195a0ba35 100644 --- a/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll +++ b/ql/src/semmle/go/dataflow/internal/DataFlowPrivate.qll @@ -141,6 +141,13 @@ predicate readStep(Node node1, Content f, Node node2) { ) } +/** + * Holds if values stored inside content `c` are cleared at node `n`. + */ +predicate clearsContent(Node n, Content c) { + none() // stub implementation +} + /** * Gets a representative (boxed) type for `t` for the purpose of pruning * possible flow. A single type is used for all numeric types to account for diff --git a/ql/src/semmle/go/dataflow/internal/tainttracking1/TaintTrackingImpl.qll b/ql/src/semmle/go/dataflow/internal/tainttracking1/TaintTrackingImpl.qll index 0f0607662e9..af0d0fec53a 100644 --- a/ql/src/semmle/go/dataflow/internal/tainttracking1/TaintTrackingImpl.qll +++ b/ql/src/semmle/go/dataflow/internal/tainttracking1/TaintTrackingImpl.qll @@ -26,7 +26,7 @@ private import TaintTrackingParameter::Private * To create a configuration, extend this class with a subclass whose * characteristic predicate is a unique singleton string. For example, write * - * ``` + * ```ql * class MyAnalysisConfiguration extends TaintTracking::Configuration { * MyAnalysisConfiguration() { this = "MyAnalysisConfiguration" } * // Override `isSource` and `isSink`. @@ -41,7 +41,7 @@ private import TaintTrackingParameter::Private * Then, to query whether there is flow between some `source` and `sink`, * write * - * ``` + * ```ql * exists(MyAnalysisConfiguration cfg | cfg.hasFlow(source, sink)) * ``` *