Skip to content

Commit 219ea28

Browse files
authored
Merge pull request #21400 from michaelnebel/csharp/implicitconversionreverseflowtaint
C#: Add default taint step from an implicit operator call to its argument.
2 parents 2782d90 + fbf40ef commit 219ea28

File tree

5 files changed

+75
-0
lines changed

5 files changed

+75
-0
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added reverse taint flow from implicit conversion operator calls to their arguments.

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,16 @@ private class LocalTaintExprStepConfiguration extends ControlFlowReachabilityCon
109109
}
110110
}
111111

112+
private ControlFlow::Nodes::ExprNode getALastEvalNode(ControlFlow::Nodes::ExprNode cfn) {
113+
exists(OperatorCall oc | any(LocalTaintExprStepConfiguration x).hasExprPath(_, result, oc, cfn) |
114+
oc.getTarget() instanceof ImplicitConversionOperator
115+
)
116+
}
117+
118+
private ControlFlow::Nodes::ExprNode getPostUpdateReverseStep(ControlFlow::Nodes::ExprNode e) {
119+
result = getALastEvalNode(e)
120+
}
121+
112122
private predicate localTaintStepCommon(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
113123
hasNodePath(any(LocalTaintExprStepConfiguration x), nodeFrom, nodeTo)
114124
}
@@ -177,6 +187,16 @@ private module Cached {
177187
readStep(nodeFrom, any(DataFlow::ContentSet c | c.isElement()), nodeTo)
178188
or
179189
nodeTo = nodeFrom.(DataFlow::NonLocalJumpNode).getAJumpSuccessor(false)
190+
or
191+
// Allow reverse update flow for implicit conversion operator calls.
192+
// This is needed to support flow out of method call arguments, where an implicit conversion is applied
193+
// to a call argument.
194+
nodeTo.(PostUpdateNode).getPreUpdateNode().(DataFlow::ExprNode).getControlFlowNode() =
195+
getPostUpdateReverseStep(nodeFrom
196+
.(PostUpdateNode)
197+
.getPreUpdateNode()
198+
.(DataFlow::ExprNode)
199+
.getControlFlowNode())
180200
) and
181201
model = ""
182202
or
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using System;
2+
3+
public class TestImplicitConversionOperator
4+
{
5+
static void Sink(object o) { }
6+
static void TaintArgument(ArraySegment<byte> segment) { }
7+
8+
public void M1()
9+
{
10+
byte[] bytes = new byte[1];
11+
TaintArgument(bytes);
12+
Sink(bytes);
13+
}
14+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
edges
2+
| ImplicitConversionOperator.cs:11:23:11:27 | [post] call to operator implicit conversion : ArraySegment<Byte> | ImplicitConversionOperator.cs:12:14:12:18 | access to local variable bytes | provenance | |
3+
nodes
4+
| ImplicitConversionOperator.cs:11:23:11:27 | [post] call to operator implicit conversion : ArraySegment<Byte> | semmle.label | [post] call to operator implicit conversion : ArraySegment<Byte> |
5+
| ImplicitConversionOperator.cs:12:14:12:18 | access to local variable bytes | semmle.label | access to local variable bytes |
6+
subpaths
7+
#select
8+
| ImplicitConversionOperator.cs:12:14:12:18 | access to local variable bytes | ImplicitConversionOperator.cs:11:23:11:27 | [post] call to operator implicit conversion : ArraySegment<Byte> | ImplicitConversionOperator.cs:12:14:12:18 | access to local variable bytes | $@ | ImplicitConversionOperator.cs:11:23:11:27 | [post] call to operator implicit conversion : ArraySegment<Byte> | [post] call to operator implicit conversion : ArraySegment<Byte> |
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @kind path-problem
3+
*/
4+
5+
import csharp
6+
import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
7+
import Taint::PathGraph
8+
9+
module TaintConfig implements DataFlow::ConfigSig {
10+
predicate isSource(DataFlow::Node src) {
11+
exists(MethodCall mc |
12+
mc.getTarget().hasName("TaintArgument") and
13+
mc.getAnArgument() = src.(DataFlowPrivate::PostUpdateNode).getPreUpdateNode().asExpr()
14+
)
15+
}
16+
17+
predicate isSink(DataFlow::Node sink) {
18+
exists(MethodCall mc |
19+
mc.getTarget().hasName("Sink") and
20+
mc.getAnArgument() = sink.asExpr()
21+
)
22+
}
23+
}
24+
25+
module Taint = TaintTracking::Global<TaintConfig>;
26+
27+
from Taint::PathNode source, Taint::PathNode sink
28+
where Taint::flowPath(source, sink)
29+
select sink, source, sink, "$@", source, source.toString()

0 commit comments

Comments
 (0)