Skip to content

Commit 19e3d7c

Browse files
hvitvedhvitved
authored
Merge pull request github#10769 from hvitved/csharp/cil-ssa-data-flow-nodes
C#: Include CIL SSA definitions in `DataFlow::Node`
2 parents d79a7e8 + d42c74f commit 19e3d7c

File tree

9 files changed

+148
-47
lines changed

9 files changed

+148
-47
lines changed

‎csharp/ql/lib/semmle/code/cil/DataFlow.qll

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,18 @@ class DataFlowNode extends @cil_dataflow_node {
2020
* Holds if this node flows to `sink` in one step.
2121
* `tt` is the tainting that occurs during this step.
2222
*/
23-
predicate getALocalFlowSucc(DataFlowNode sink, TaintType tt) {
23+
deprecated predicate getALocalFlowSucc(DataFlowNode sink, TaintType tt) {
2424
localExactStep(this, sink) and tt = TExactValue()
2525
or
2626
localTaintStep(this, sink) and tt = TTaintedValue()
2727
}
2828

29-
private predicate flowsToStep(DataFlowNode sink) { this.getALocalFlowSucc(sink, TExactValue()) }
29+
deprecated private predicate flowsToStep(DataFlowNode sink) {
30+
this.getALocalFlowSucc(sink, TExactValue())
31+
}
3032

3133
/** Holds if this node flows to `sink` in zero or more steps. */
32-
predicate flowsTo(DataFlowNode sink) { this.flowsToStep*(sink) }
34+
deprecated predicate flowsTo(DataFlowNode sink) { this.flowsToStep*(sink) }
3335

3436
/** Gets the method that contains this dataflow node. */
3537
Method getMethod() { none() }
@@ -38,12 +40,12 @@ class DataFlowNode extends @cil_dataflow_node {
3840
Location getLocation() { none() }
3941
}
4042

41-
private newtype TTaintType =
43+
deprecated private newtype TTaintType =
4244
TExactValue() or
4345
TTaintedValue()
4446

4547
/** Describes how data is tainted. */
46-
class TaintType extends TTaintType {
48+
deprecated class TaintType extends TTaintType {
4749
string toString() {
4850
this = TExactValue() and result = "exact"
4951
or
@@ -52,12 +54,12 @@ class TaintType extends TTaintType {
5254
}
5355

5456
/** A taint type where the data is untainted. */
55-
class Untainted extends TaintType, TExactValue { }
57+
deprecated class Untainted extends TaintType, TExactValue { }
5658

5759
/** A taint type where the data is tainted. */
58-
class Tainted extends TaintType, TTaintedValue { }
60+
deprecated class Tainted extends TaintType, TTaintedValue { }
5961

60-
private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
62+
deprecated private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
6163
exists(Ssa::Definition def, BasicBlock bb, int i | phi.hasLastInputRef(def, bb, i) |
6264
def.definesAt(_, bb, i) and
6365
input = def.getVariableUpdate().getSource()
@@ -76,7 +78,7 @@ private predicate localFlowPhiInput(DataFlowNode input, Ssa::PhiNode phi) {
7678
)
7779
}
7880

79-
private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
81+
deprecated private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
8082
src = sink.(Opcodes::Dup).getAnOperand()
8183
or
8284
exists(Ssa::Definition def, VariableUpdate vu |
@@ -103,7 +105,7 @@ private predicate localExactStep(DataFlowNode src, DataFlowNode sink) {
103105
src = sink.(ConditionalBranch).getAnOperand()
104106
}
105107

106-
private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
108+
deprecated private predicate localTaintStep(DataFlowNode src, DataFlowNode sink) {
107109
src = sink.(BinaryArithmeticExpr).getAnOperand() or
108110
src = sink.(Opcodes::Neg).getOperand() or
109111
src = sink.(UnaryBitwiseOperation).getOperand()

‎csharp/ql/lib/semmle/code/cil/Method.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ class Setter extends Accessor {
270270
*/
271271
class TrivialSetter extends Method {
272272
TrivialSetter() {
273-
exists(MethodImplementation impl | impl = this.getImplementation() |
273+
exists(MethodImplementation impl | impl = this.getAnImplementation() |
274274
impl.getInstruction(0) instanceof ThisAccess and
275275
impl.getInstruction(1).(ParameterReadAccess).getTarget().getIndex() = 1 and
276276
impl.getInstruction(2) instanceof FieldWriteAccess

‎csharp/ql/lib/semmle/code/cil/Ssa.qll

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ module Ssa {
2424
}
2525

2626
/** Gets a first read of this SSA definition. */
27-
final ReadAccess getAFirstRead() { result = SsaImpl::getAFirstRead(this) }
27+
deprecated final ReadAccess getAFirstRead() { result = SsaImpl::getAFirstRead(this) }
2828

2929
/** Holds if `first` and `second` are adjacent reads of this SSA definition. */
30-
final predicate hasAdjacentReads(ReadAccess first, ReadAccess second) {
30+
deprecated final predicate hasAdjacentReads(ReadAccess first, ReadAccess second) {
3131
SsaImpl::hasAdjacentReads(this, first, second)
3232
}
3333

@@ -58,8 +58,9 @@ module Ssa {
5858
* index `i` in basic block `bb` can reach this phi node without going through
5959
* other references.
6060
*/
61-
final predicate hasLastInputRef(Definition def, BasicBlock bb, int i) {
62-
SsaImpl::hasLastInputRef(this, def, bb, i)
61+
deprecated final predicate hasLastInputRef(Definition def, BasicBlock bb, int i) {
62+
SsaImpl::lastRefRedef(def, bb, i, this) and
63+
def = SsaImpl::getAPhiInput(this)
6364
}
6465
}
6566
}

‎csharp/ql/lib/semmle/code/cil/Stubs.qll

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,17 @@ private module Cached {
2929

3030
cached
3131
predicate bestImplementation(MethodImplementation mi) {
32-
not assemblyIsStubImpl(mi.getLocation()) and
33-
not exists(MethodImplementation better | mi.getMethod() = better.getMethod() |
34-
mi.getNumberOfInstructions() < better.getNumberOfInstructions()
35-
or
36-
mi.getNumberOfInstructions() = better.getNumberOfInstructions() and
37-
mi.getLocation().getFile().toString() > better.getLocation().getFile().toString()
38-
) and
39-
exists(mi.getAnInstruction())
32+
exists(Assembly asm |
33+
asm = mi.getLocation() and
34+
(assemblyIsStubImpl(asm) implies asm.getFile().extractedQlTest()) and
35+
not exists(MethodImplementation better | mi.getMethod() = better.getMethod() |
36+
mi.getNumberOfInstructions() < better.getNumberOfInstructions()
37+
or
38+
mi.getNumberOfInstructions() = better.getNumberOfInstructions() and
39+
asm.getFile().toString() > better.getLocation().getFile().toString()
40+
) and
41+
exists(mi.getAnInstruction())
42+
)
4043
}
4144
}
4245

‎csharp/ql/lib/semmle/code/cil/internal/SsaImpl.qll

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ private module Cached {
6868
Definition getAPhiInput(PhiNode phi) { phiHasInputFromBlock(phi, result, _) }
6969

7070
cached
71-
predicate hasLastInputRef(Definition phi, Definition def, BasicBlock bb, int i) {
72-
lastRefRedef(def, bb, i, phi) and
73-
def = getAPhiInput(phi)
71+
predicate lastRefBeforeRedef(Definition def, BasicBlock bb, int i, Definition next) {
72+
lastRefRedef(def, bb, i, next)
7473
}
7574
}
7675

‎csharp/ql/lib/semmle/code/csharp/commons/Disposal.qll

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,19 @@ private predicate isDisposeMethod(DotNet::Callable method) {
1111
method.getNumberOfParameters() = 0
1212
}
1313

14-
private predicate cilVariableReadFlowsTo(CIL::Variable variable, CIL::DataFlowNode n) {
15-
n = variable.getARead()
14+
private predicate cilVariableReadFlowsToNode(CIL::Variable variable, DataFlow::Node n) {
15+
n.asExpr() = variable.getARead()
1616
or
17-
exists(CIL::DataFlowNode mid |
18-
cilVariableReadFlowsTo(variable, mid) and
19-
mid.getALocalFlowSucc(n, any(CIL::Untainted u))
17+
exists(DataFlow::Node mid |
18+
cilVariableReadFlowsToNode(variable, mid) and
19+
DataFlow::localFlowStep(mid, n)
2020
)
2121
}
2222

23+
private predicate cilVariableReadFlowsTo(CIL::Variable variable, CIL::DataFlowNode n) {
24+
cilVariableReadFlowsToNode(variable, DataFlow::exprNode(n))
25+
}
26+
2327
private predicate disposedCilVariable(CIL::Variable variable) {
2428
// `variable` is the `this` parameter on a dispose method.
2529
isDisposeMethod(variable.(CIL::ThisParameter).getMethod())

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

Lines changed: 102 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ private import semmle.code.csharp.frameworks.EntityFramework
1717
private import semmle.code.csharp.frameworks.NHibernate
1818
private import semmle.code.csharp.frameworks.system.Collections
1919
private import semmle.code.csharp.frameworks.system.threading.Tasks
20+
private import semmle.code.cil.Ssa::Ssa as CilSsa
2021

2122
/** Gets the callable in which this node occurs. */
2223
DataFlowCallable nodeGetEnclosingCallable(NodeImpl n) { result = n.getEnclosingCallableImpl() }
@@ -177,6 +178,12 @@ predicate hasNodePath(ControlFlowReachabilityConfiguration conf, ExprNode n1, No
177178
)
178179
}
179180

181+
/** Gets the CIL data-flow node for `node`, if any. */
182+
CIL::DataFlowNode asCilDataFlowNode(Node node) {
183+
result = node.asParameter() or
184+
result = node.asExpr()
185+
}
186+
180187
/** Provides predicates related to local data flow. */
181188
module LocalFlow {
182189
private class LocalExprStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -281,15 +288,6 @@ module LocalFlow {
281288
}
282289
}
283290

284-
private CIL::DataFlowNode asCilDataFlowNode(Node node) {
285-
result = node.asParameter() or
286-
result = node.asExpr()
287-
}
288-
289-
private predicate localFlowStepCil(Node nodeFrom, Node nodeTo) {
290-
asCilDataFlowNode(nodeFrom).getALocalFlowSucc(asCilDataFlowNode(nodeTo), any(CIL::Untainted t))
291-
}
292-
293291
/**
294292
* An uncertain SSA definition. Either an uncertain explicit definition or an
295293
* uncertain qualifier definition.
@@ -341,7 +339,7 @@ module LocalFlow {
341339

342340
/**
343341
* Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
344-
* SSA definition `def.
342+
* SSA definition `def`.
345343
*/
346344
predicate localSsaFlowStep(Ssa::Definition def, Node nodeFrom, Node nodeTo) {
347345
// Flow from SSA definition/parameter to first read
@@ -386,6 +384,76 @@ module LocalFlow {
386384
)
387385
}
388386

387+
private module CilFlow {
388+
private import semmle.code.cil.internal.SsaImpl as CilSsaImpl
389+
390+
/**
391+
* Holds if `nodeFrom` is a last node referencing SSA definition `def`, which
392+
* can reach `next`.
393+
*/
394+
private predicate localFlowCilSsaInput(
395+
Node nodeFrom, CilSsa::Definition def, CilSsa::Definition next
396+
) {
397+
exists(CIL::BasicBlock bb, int i | CilSsaImpl::lastRefBeforeRedef(def, bb, i, next) |
398+
def.definesAt(_, bb, i) and
399+
def = nodeFrom.(CilSsaDefinitionNode).getDefinition()
400+
or
401+
nodeFrom = TCilExprNode(bb.getNode(i).(CIL::ReadAccess))
402+
)
403+
}
404+
405+
/**
406+
* Holds if there is a local flow step from `nodeFrom` to `nodeTo` involving
407+
* CIL SSA definition `def`.
408+
*/
409+
private predicate localCilSsaFlowStep(CilSsa::Definition def, Node nodeFrom, Node nodeTo) {
410+
// Flow into SSA definition
411+
exists(CIL::VariableUpdate vu |
412+
vu = def.getVariableUpdate() and
413+
vu.getSource() = asCilDataFlowNode(nodeFrom) and
414+
def = nodeTo.(CilSsaDefinitionNode).getDefinition()
415+
)
416+
or
417+
// Flow from SSA definition to first read
418+
def = nodeFrom.(CilSsaDefinitionNode).getDefinition() and
419+
nodeTo = TCilExprNode(CilSsaImpl::getAFirstRead(def))
420+
or
421+
// Flow from read to next read
422+
exists(CIL::ReadAccess readFrom, CIL::ReadAccess readTo |
423+
CilSsaImpl::hasAdjacentReads(def, readFrom, readTo) and
424+
nodeTo = TCilExprNode(readTo) and
425+
nodeFrom = TCilExprNode(readFrom)
426+
)
427+
or
428+
// Flow into phi node
429+
exists(CilSsa::PhiNode phi |
430+
localFlowCilSsaInput(nodeFrom, def, phi) and
431+
phi = nodeTo.(CilSsaDefinitionNode).getDefinition() and
432+
def = CilSsaImpl::getAPhiInput(phi)
433+
)
434+
}
435+
436+
private predicate localExactStep(CIL::DataFlowNode src, CIL::DataFlowNode sink) {
437+
src = sink.(CIL::Opcodes::Dup).getAnOperand()
438+
or
439+
src = sink.(CIL::Conversion).getExpr()
440+
or
441+
src = sink.(CIL::WriteAccess).getExpr()
442+
or
443+
src = sink.(CIL::Method).getAnImplementation().getAnInstruction().(CIL::Return)
444+
or
445+
src = sink.(CIL::Return).getExpr()
446+
or
447+
src = sink.(CIL::ConditionalBranch).getAnOperand()
448+
}
449+
450+
predicate localFlowStepCil(Node nodeFrom, Node nodeTo) {
451+
localExactStep(asCilDataFlowNode(nodeFrom), asCilDataFlowNode(nodeTo))
452+
or
453+
localCilSsaFlowStep(_, nodeFrom, nodeTo)
454+
}
455+
}
456+
389457
predicate localFlowStepCommon(Node nodeFrom, Node nodeTo) {
390458
exists(Ssa::Definition def |
391459
localSsaFlowStep(def, nodeFrom, nodeTo) and
@@ -398,7 +466,7 @@ module LocalFlow {
398466
or
399467
ThisFlow::adjacentThisRefs(nodeFrom.(PostUpdateNode).getPreUpdateNode(), nodeTo)
400468
or
401-
localFlowStepCil(nodeFrom, nodeTo)
469+
CilFlow::localFlowStepCil(nodeFrom, nodeTo)
402470
}
403471

404472
/**
@@ -719,6 +787,7 @@ private module Cached {
719787
cfn.getElement() instanceof Expr
720788
} or
721789
TCilExprNode(CIL::Expr e) { e.getImplementation() instanceof CIL::BestImplementation } or
790+
TCilSsaDefinitionNode(CilSsa::Definition def) or
722791
TSsaDefinitionNode(Ssa::Definition def) {
723792
// Handled by `TExplicitParameterNode` below
724793
not def.(Ssa::ExplicitDefinition).getADefinition() instanceof
@@ -867,6 +936,28 @@ predicate nodeIsHidden(Node n) {
867936
n.asExpr() = any(WithExpr we).getInitializer()
868937
}
869938

939+
/** A CIL SSA definition, viewed as a node in a data flow graph. */
940+
class CilSsaDefinitionNode extends NodeImpl, TCilSsaDefinitionNode {
941+
CilSsa::Definition def;
942+
943+
CilSsaDefinitionNode() { this = TCilSsaDefinitionNode(def) }
944+
945+
/** Gets the underlying SSA definition. */
946+
CilSsa::Definition getDefinition() { result = def }
947+
948+
override DataFlowCallable getEnclosingCallableImpl() {
949+
result.asCallable() = def.getBasicBlock().getFirstNode().getImplementation().getMethod()
950+
}
951+
952+
override CIL::Type getTypeImpl() { result = def.getSourceVariable().getType() }
953+
954+
override ControlFlow::Node getControlFlowNodeImpl() { none() }
955+
956+
override Location getLocationImpl() { result = def.getBasicBlock().getLocation() }
957+
958+
override string toStringImpl() { result = def.toString() }
959+
}
960+
870961
/** An SSA definition, viewed as a node in a data flow graph. */
871962
class SsaDefinitionNode extends NodeImpl, TSsaDefinitionNode {
872963
Ssa::Definition def;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ predicate localFlow(Node source, Node sink) { localFlowStep*(source, sink) }
161161
* local (intra-procedural) steps.
162162
*/
163163
pragma[inline]
164-
predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
164+
predicate localExprFlow(DotNet::Expr e1, DotNet::Expr e2) { localFlow(exprNode(e1), exprNode(e2)) }
165165

166166
/**
167167
* A data flow node that jumps between callables. This can be extended in

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ predicate defaultTaintSanitizer(DataFlow::Node node) { none() }
2626
bindingset[node]
2727
predicate defaultImplicitTaintRead(DataFlow::Node node, DataFlow::Content c) { none() }
2828

29-
private CIL::DataFlowNode asCilDataFlowNode(DataFlow::Node node) {
30-
result = node.asParameter() or
31-
result = node.asExpr()
29+
private predicate localCilTaintStep(CIL::DataFlowNode src, CIL::DataFlowNode sink) {
30+
src = sink.(CIL::BinaryArithmeticExpr).getAnOperand() or
31+
src = sink.(CIL::Opcodes::Neg).getOperand() or
32+
src = sink.(CIL::UnaryBitwiseOperation).getOperand()
3233
}
3334

3435
private predicate localTaintStepCil(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
35-
asCilDataFlowNode(nodeFrom).getALocalFlowSucc(asCilDataFlowNode(nodeTo), any(CIL::Tainted t))
36+
localCilTaintStep(asCilDataFlowNode(nodeFrom), asCilDataFlowNode(nodeTo))
3637
}
3738

3839
private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityConfiguration {

0 commit comments

Comments
 (0)