Skip to content

Commit a1d5591

Browse files
committed
C#: Model field-clearing in data-flow
1 parent b5bc15a commit a1d5591

File tree

5 files changed

+63
-73
lines changed

5 files changed

+63
-73
lines changed

csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImpl.qll

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,13 @@ private predicate flowIntoCallNodeCand2(
10511051
}
10521052

10531053
private module LocalFlowBigStep {
1054+
private class BigStepBarrierNode extends Node {
1055+
BigStepBarrierNode() {
1056+
this instanceof CastNode or
1057+
clearsContent(this, _)
1058+
}
1059+
}
1060+
10541061
/**
10551062
* Holds if `node` can be the first node in a maximal subsequence of local
10561063
* flow steps in a dataflow path.
@@ -1065,7 +1072,7 @@ private module LocalFlowBigStep {
10651072
node instanceof OutNodeExt or
10661073
store(_, _, node, _) or
10671074
read(_, _, node) or
1068-
node instanceof CastNode
1075+
node instanceof BigStepBarrierNode
10691076
)
10701077
}
10711078

@@ -1083,7 +1090,7 @@ private module LocalFlowBigStep {
10831090
read(node, _, next)
10841091
)
10851092
or
1086-
node instanceof CastNode
1093+
node instanceof BigStepBarrierNode
10871094
or
10881095
config.isSink(node)
10891096
}
@@ -1127,14 +1134,14 @@ private module LocalFlowBigStep {
11271134
exists(Node mid |
11281135
localFlowStepPlus(node1, mid, preservesValue, t, config, cc) and
11291136
localFlowStepNodeCand1(mid, node2, config) and
1130-
not mid instanceof CastNode and
1137+
not mid instanceof BigStepBarrierNode and
11311138
nodeCand2(node2, unbind(config))
11321139
)
11331140
or
11341141
exists(Node mid |
11351142
localFlowStepPlus(node1, mid, _, _, config, cc) and
11361143
additionalLocalFlowStepNodeCand2(mid, node2, config) and
1137-
not mid instanceof CastNode and
1144+
not mid instanceof BigStepBarrierNode and
11381145
preservesValue = false and
11391146
t = getErasedNodeTypeBound(node2) and
11401147
nodeCand2(node2, unbind(config))
@@ -1208,7 +1215,8 @@ private predicate flowCandFwd0(
12081215
or
12091216
exists(Node mid |
12101217
flowCandFwd(mid, fromArg, argApf, apf, config) and
1211-
localFlowBigStep(mid, node, true, _, config, _)
1218+
localFlowBigStep(mid, node, true, _, config, _) and
1219+
not apf.isClearedAt(node)
12121220
)
12131221
or
12141222
exists(Node mid, AccessPathFrontNil nil |
@@ -1221,7 +1229,8 @@ private predicate flowCandFwd0(
12211229
nodeCand2(node, unbind(config)) and
12221230
jumpStep(mid, node, config) and
12231231
fromArg = false and
1224-
argApf = TAccessPathFrontNone()
1232+
argApf = TAccessPathFrontNone() and
1233+
not apf.isClearedAt(node)
12251234
)
12261235
or
12271236
exists(Node mid, AccessPathFrontNil nil |
@@ -1246,7 +1255,8 @@ private predicate flowCandFwd0(
12461255
exists(TypedContent tc |
12471256
flowCandFwdRead(tc, node, fromArg, argApf, config) and
12481257
flowCandFwdConsCand(tc, apf, config) and
1249-
nodeCand2(node, _, _, unbindBool(apf.toBoolNonEmpty()), unbind(config))
1258+
nodeCand2(node, _, _, unbindBool(apf.toBoolNonEmpty()), unbind(config)) and
1259+
not apf.isClearedAt(node)
12501260
)
12511261
or
12521262
// flow into a callable
@@ -1302,7 +1312,8 @@ private predicate flowCandFwdIn(
13021312
) {
13031313
exists(ArgumentNode arg, boolean allowsFieldFlow |
13041314
flowCandFwd(arg, fromArg, argApf, apf, config) and
1305-
flowIntoCallNodeCand2(call, arg, p, allowsFieldFlow, config)
1315+
flowIntoCallNodeCand2(call, arg, p, allowsFieldFlow, config) and
1316+
not apf.isClearedAt(p)
13061317
|
13071318
apf instanceof AccessPathFrontNil or allowsFieldFlow = true
13081319
)
@@ -1315,7 +1326,8 @@ private predicate flowCandFwdOut(
13151326
) {
13161327
exists(ReturnNodeExt ret, boolean allowsFieldFlow |
13171328
flowCandFwd(ret, fromArg, argApf, apf, config) and
1318-
flowOutOfCallNodeCand2(call, ret, node, allowsFieldFlow, config)
1329+
flowOutOfCallNodeCand2(call, ret, node, allowsFieldFlow, config) and
1330+
not apf.isClearedAt(node)
13191331
|
13201332
apf instanceof AccessPathFrontNil or allowsFieldFlow = true
13211333
)

csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -754,6 +754,13 @@ abstract class AccessPathFront extends TAccessPathFront {
754754
abstract boolean toBoolNonEmpty();
755755

756756
predicate headUsesContent(TypedContent tc) { this = TFrontHead(tc) }
757+
758+
predicate isClearedAt(Node n) {
759+
exists(TypedContent tc |
760+
this.headUsesContent(tc) and
761+
clearsContent(n, tc.getContent())
762+
)
763+
}
757764
}
758765

759766
class AccessPathFrontNil extends AccessPathFront, TFrontNil {

csharp/ql/src/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,20 @@ private module Cached {
547547
node1 = node2.(LibraryCodeNode).getPredecessor(any(AccessPath ap | ap.getHead() = c))
548548
}
549549

550+
/**
551+
* Holds if values stored inside content `c` are cleared at node `n`. For example,
552+
* any value stored inside `f` is cleared at the pre-update node associated with `x`
553+
* in `x.f = newValue`.
554+
*/
555+
cached
556+
predicate clearsContent(Node n, Content c) {
557+
fieldOrPropertyAssign(_, c, _, n.asExpr())
558+
or
559+
fieldOrPropertyInit(n.asExpr(), c, _)
560+
or
561+
exists(n.(LibraryCodeNode).getSuccessor(any(AccessPath ap | ap.getHead() = c)))
562+
}
563+
550564
/**
551565
* Holds if the node `n` is unreachable when the call context is `call`.
552566
*/

0 commit comments

Comments
 (0)