Skip to content

Commit 00ee34c

Browse files
aschackmullaschackmull
authored
Merge pull request #7237 from hvitved/dataflow/consistency-config
Data flow: Introduce `ConsistencyConfiguration` class
2 parents 57fd397 + 6cb0099 commit 00ee34c

File tree

13 files changed

+136
-82
lines changed

13 files changed

+136
-82
lines changed

‎cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
99
private import tainttracking1.TaintTrackingParameter::Public
1010

1111
module Consistency {
12+
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
13+
14+
/** A class for configuring the consistency queries. */
15+
class ConsistencyConfiguration extends TConsistencyConfiguration {
16+
string toString() { none() }
17+
18+
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
19+
predicate postWithInFlowExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
22+
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
23+
}
24+
1225
private class RelevantNode extends Node {
1326
RelevantNode() {
1427
this instanceof ArgumentNode or
@@ -164,7 +177,7 @@ module Consistency {
164177

165178
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
166179
not hasPost(n) and
167-
not isImmutableOrUnobservable(n) and
180+
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
168181
msg = "ArgumentNode is missing PostUpdateNode."
169182
}
170183

@@ -177,6 +190,7 @@ module Consistency {
177190
isPostUpdateNode(n) and
178191
not clearsContent(n, _) and
179192
simpleLocalFlowStep(_, n) and
193+
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
180194
msg = "PostUpdateNode should not be the target of local flow."
181195
}
182196
}

‎cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ private import cpp
22
private import DataFlowUtil
33
private import DataFlowDispatch
44
private import FlowVar
5+
private import DataFlowImplConsistency
56

67
/** Gets the callable in which this node occurs. */
78
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
@@ -259,27 +260,6 @@ class Unit extends TUnit {
259260
string toString() { result = "unit" }
260261
}
261262

262-
/**
263-
* Holds if `n` does not require a `PostUpdateNode` as it either cannot be
264-
* modified or its modification cannot be observed, for example if it is a
265-
* freshly created object that is not saved in a variable.
266-
*
267-
* This predicate is only used for consistency checks.
268-
*/
269-
predicate isImmutableOrUnobservable(Node n) {
270-
// Is the null pointer (or something that's not really a pointer)
271-
exists(n.asExpr().getValue())
272-
or
273-
// Isn't a pointer or is a pointer to const
274-
forall(DerivedType dt | dt = n.asExpr().getActualType() |
275-
dt.getBaseType().isConst()
276-
or
277-
dt.getBaseType() instanceof RoutineType
278-
)
279-
// The above list of cases isn't exhaustive, but it narrows down the
280-
// consistency alerts enough that most of them are interesting.
281-
}
282-
283263
/** Holds if `n` should be hidden from path explanations. */
284264
predicate nodeIsHidden(Node n) { none() }
285265

@@ -302,3 +282,19 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
302282
* by default as a heuristic.
303283
*/
304284
predicate allowParameterReturnInSelf(ParameterNode p) { none() }
285+
286+
private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
287+
override predicate argHasPostUpdateExclude(ArgumentNode n) {
288+
// Is the null pointer (or something that's not really a pointer)
289+
exists(n.asExpr().getValue())
290+
or
291+
// Isn't a pointer or is a pointer to const
292+
forall(DerivedType dt | dt = n.asExpr().getActualType() |
293+
dt.getBaseType().isConst()
294+
or
295+
dt.getBaseType() instanceof RoutineType
296+
)
297+
// The above list of cases isn't exhaustive, but it narrows down the
298+
// consistency alerts enough that most of them are interesting.
299+
}
300+
}

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
99
private import tainttracking1.TaintTrackingParameter::Public
1010

1111
module Consistency {
12+
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
13+
14+
/** A class for configuring the consistency queries. */
15+
class ConsistencyConfiguration extends TConsistencyConfiguration {
16+
string toString() { none() }
17+
18+
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
19+
predicate postWithInFlowExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
22+
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
23+
}
24+
1225
private class RelevantNode extends Node {
1326
RelevantNode() {
1427
this instanceof ArgumentNode or
@@ -164,7 +177,7 @@ module Consistency {
164177

165178
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
166179
not hasPost(n) and
167-
not isImmutableOrUnobservable(n) and
180+
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
168181
msg = "ArgumentNode is missing PostUpdateNode."
169182
}
170183

@@ -177,6 +190,7 @@ module Consistency {
177190
isPostUpdateNode(n) and
178191
not clearsContent(n, _) and
179192
simpleLocalFlowStep(_, n) and
193+
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
180194
msg = "PostUpdateNode should not be the target of local flow."
181195
}
182196
}

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ private import cpp
22
private import DataFlowUtil
33
private import semmle.code.cpp.ir.IR
44
private import DataFlowDispatch
5+
private import DataFlowImplConsistency
56

67
/** Gets the callable in which this node occurs. */
78
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }
@@ -285,19 +286,6 @@ class Unit extends TUnit {
285286
string toString() { result = "unit" }
286287
}
287288

288-
/**
289-
* Holds if `n` does not require a `PostUpdateNode` as it either cannot be
290-
* modified or its modification cannot be observed, for example if it is a
291-
* freshly created object that is not saved in a variable.
292-
*
293-
* This predicate is only used for consistency checks.
294-
*/
295-
predicate isImmutableOrUnobservable(Node n) {
296-
// The rules for whether an IR argument gets a post-update node are too
297-
// complex to model here.
298-
any()
299-
}
300-
301289
/** Holds if `n` should be hidden from path explanations. */
302290
predicate nodeIsHidden(Node n) {
303291
n instanceof OperandNode and not n instanceof ArgumentNode
@@ -330,3 +318,11 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
330318
* by default as a heuristic.
331319
*/
332320
predicate allowParameterReturnInSelf(ParameterNode p) { none() }
321+
322+
private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
323+
override predicate argHasPostUpdateExclude(ArgumentNode n) {
324+
// The rules for whether an IR argument gets a post-update node are too
325+
// complex to model here.
326+
any()
327+
}
328+
}

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
99
private import tainttracking1.TaintTrackingParameter::Public
1010

1111
module Consistency {
12+
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
13+
14+
/** A class for configuring the consistency queries. */
15+
class ConsistencyConfiguration extends TConsistencyConfiguration {
16+
string toString() { none() }
17+
18+
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
19+
predicate postWithInFlowExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
22+
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
23+
}
24+
1225
private class RelevantNode extends Node {
1326
RelevantNode() {
1427
this instanceof ArgumentNode or
@@ -164,7 +177,7 @@ module Consistency {
164177

165178
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
166179
not hasPost(n) and
167-
not isImmutableOrUnobservable(n) and
180+
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
168181
msg = "ArgumentNode is missing PostUpdateNode."
169182
}
170183

@@ -177,6 +190,7 @@ module Consistency {
177190
isPostUpdateNode(n) and
178191
not clearsContent(n, _) and
179192
simpleLocalFlowStep(_, n) and
193+
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
180194
msg = "PostUpdateNode should not be the target of local flow."
181195
}
182196
}

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1947,15 +1947,6 @@ class Unit extends TUnit {
19471947
string toString() { result = "unit" }
19481948
}
19491949

1950-
/**
1951-
* Holds if `n` does not require a `PostUpdateNode` as it either cannot be
1952-
* modified or its modification cannot be observed, for example if it is a
1953-
* freshly created object that is not saved in a variable.
1954-
*
1955-
* This predicate is only used for consistency checks.
1956-
*/
1957-
predicate isImmutableOrUnobservable(Node n) { none() }
1958-
19591950
class LambdaCallKind = Unit;
19601951

19611952
/** Holds if `creation` is an expression that creates a delegate for `c`. */

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
99
private import tainttracking1.TaintTrackingParameter::Public
1010

1111
module Consistency {
12+
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
13+
14+
/** A class for configuring the consistency queries. */
15+
class ConsistencyConfiguration extends TConsistencyConfiguration {
16+
string toString() { none() }
17+
18+
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
19+
predicate postWithInFlowExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
22+
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
23+
}
24+
1225
private class RelevantNode extends Node {
1326
RelevantNode() {
1427
this instanceof ArgumentNode or
@@ -164,7 +177,7 @@ module Consistency {
164177

165178
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
166179
not hasPost(n) and
167-
not isImmutableOrUnobservable(n) and
180+
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
168181
msg = "ArgumentNode is missing PostUpdateNode."
169182
}
170183

@@ -177,6 +190,7 @@ module Consistency {
177190
isPostUpdateNode(n) and
178191
not clearsContent(n, _) and
179192
simpleLocalFlowStep(_, n) and
193+
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
180194
msg = "PostUpdateNode should not be the target of local flow."
181195
}
182196
}

‎java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ private import ContainerFlow
88
private import semmle.code.java.dataflow.FlowSteps
99
private import semmle.code.java.dataflow.FlowSummary
1010
private import FlowSummaryImpl as FlowSummaryImpl
11+
private import DataFlowImplConsistency
1112
import DataFlowNodes::Private
1213

1314
private newtype TReturnKind = TNormalReturnKind()
@@ -365,17 +366,6 @@ predicate forceHighPrecision(Content c) {
365366
c instanceof ArrayContent or c instanceof CollectionContent
366367
}
367368

368-
/**
369-
* Holds if `n` does not require a `PostUpdateNode` as it either cannot be
370-
* modified or its modification cannot be observed, for example if it is a
371-
* freshly created object that is not saved in a variable.
372-
*
373-
* This predicate is only used for consistency checks.
374-
*/
375-
predicate isImmutableOrUnobservable(Node n) {
376-
n.getType() instanceof ImmutableType or n instanceof ImplicitVarargsArray
377-
}
378-
379369
/** Holds if `n` should be hidden from path explanations. */
380370
predicate nodeIsHidden(Node n) {
381371
n instanceof SummaryNode
@@ -420,3 +410,9 @@ predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preserves
420410
predicate allowParameterReturnInSelf(ParameterNode p) {
421411
FlowSummaryImpl::Private::summaryAllowParameterReturnInSelf(p)
422412
}
413+
414+
private class MyConsistencyConfiguration extends Consistency::ConsistencyConfiguration {
415+
override predicate argHasPostUpdateExclude(ArgumentNode n) {
416+
n.getType() instanceof ImmutableType or n instanceof ImplicitVarargsArray
417+
}
418+
}

‎python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,19 @@ private import tainttracking1.TaintTrackingParameter::Private
99
private import tainttracking1.TaintTrackingParameter::Public
1010

1111
module Consistency {
12+
private newtype TConsistencyConfiguration = MkConsistencyConfiguration()
13+
14+
/** A class for configuring the consistency queries. */
15+
class ConsistencyConfiguration extends TConsistencyConfiguration {
16+
string toString() { none() }
17+
18+
/** Holds if `n` should be excluded from the consistency test `postWithInFlow`. */
19+
predicate postWithInFlowExclude(Node n) { none() }
20+
21+
/** Holds if `n` should be excluded from the consistency test `argHasPostUpdate`. */
22+
predicate argHasPostUpdateExclude(ArgumentNode n) { none() }
23+
}
24+
1225
private class RelevantNode extends Node {
1326
RelevantNode() {
1427
this instanceof ArgumentNode or
@@ -164,7 +177,7 @@ module Consistency {
164177

165178
query predicate argHasPostUpdate(ArgumentNode n, string msg) {
166179
not hasPost(n) and
167-
not isImmutableOrUnobservable(n) and
180+
not any(ConsistencyConfiguration c).argHasPostUpdateExclude(n) and
168181
msg = "ArgumentNode is missing PostUpdateNode."
169182
}
170183

@@ -177,6 +190,7 @@ module Consistency {
177190
isPostUpdateNode(n) and
178191
not clearsContent(n, _) and
179192
simpleLocalFlowStep(_, n) and
193+
not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
180194
msg = "PostUpdateNode should not be the target of local flow."
181195
}
182196
}

‎python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,18 +1649,6 @@ DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
16491649
*/
16501650
predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) { none() }
16511651

1652-
//--------
1653-
// Misc
1654-
//--------
1655-
/**
1656-
* Holds if `n` does not require a `PostUpdateNode` as it either cannot be
1657-
* modified or its modification cannot be observed, for example if it is a
1658-
* freshly created object that is not saved in a variable.
1659-
*
1660-
* This predicate is only used for consistency checks.
1661-
*/
1662-
predicate isImmutableOrUnobservable(Node n) { none() }
1663-
16641652
int accessPathLimit() { result = 5 }
16651653

16661654
/**

‎ruby/ql/consistency-queries/DataFlowConsistency.ql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,13 @@
1+
import codeql.ruby.DataFlow::DataFlow
2+
import codeql.ruby.dataflow.internal.DataFlowPrivate
13
import codeql.ruby.dataflow.internal.DataFlowImplConsistency::Consistency
4+
5+
private class MyConsistencyConfiguration extends ConsistencyConfiguration {
6+
override predicate postWithInFlowExclude(Node n) { n instanceof SummaryNode }
7+
8+
override predicate argHasPostUpdateExclude(ArgumentNode n) {
9+
n instanceof BlockArgumentNode
10+
or
11+
n instanceof SummaryNode
12+
}
13+
}

0 commit comments

Comments
 (0)