Skip to content

Commit 0698bdd

Browse files
hvitvedtamasvajk
hvitved
authored and
tamasvajk
committed
C#: Restrict tuple read/store steps to tuple deconstructions/constructions
1 parent 6d409a0 commit 0698bdd

File tree

5 files changed

+85
-76
lines changed

5 files changed

+85
-76
lines changed

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

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -488,13 +488,16 @@ private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, bool
488488
)
489489
or
490490
// Tuple element, `(..., src, ...)` `f` is `ItemX` of tuple `q`
491-
exists(int i |
492-
not (src instanceof LocalVariableDeclExpr or src instanceof VariableWrite) and
493-
e = q and
494-
src = q.(TupleExpr).getArgument(i) and
495-
f = q.getType().(TupleType).getElement(i) and
496-
postUpdate = false
497-
)
491+
e =
492+
any(TupleExpr te |
493+
exists(int i |
494+
e = q and
495+
src = te.getArgument(i) and
496+
te.isConstruction() and
497+
f = q.getType().(TupleType).getElement(i) and
498+
postUpdate = false
499+
)
500+
)
498501
)
499502
}
500503

@@ -807,24 +810,24 @@ private module Cached {
807810
or
808811
// node1 = (..., node2, ...)
809812
// node1.ItemX flows to node2
810-
exists(
811-
int i, Ssa::ExplicitDefinition def, AssignableDefinitions::TupleAssignmentDefinition tad,
812-
Expr item
813-
|
813+
exists(TupleExpr te, int i, Expr item |
814+
te = node1.asExpr() and
815+
not te.isConstruction() and
816+
c.(FieldContent).getField() = te.getType().(TupleType).getElement(i).getUnboundDeclaration() and
814817
// node1 = (..., item, ...)
815-
node1.asExpr().(TupleExpr).getArgument(i) = item and
816-
(
817-
// item = (..., ..., ...) in node1 = (..., (..., ..., ...), ...)
818-
node2.asExpr().(TupleExpr) = item and hasNodePath(x, node2, node1)
819-
or
820-
// item = variable in node1 = (..., variable, ...)
818+
te.getArgument(i) = item
819+
|
820+
// item = (..., ..., ...) in node1 = (..., (..., ..., ...), ...)
821+
node2.asExpr().(TupleExpr) = item and
822+
hasNodePath(x, node2, node1)
823+
or
824+
// item = variable in node1 = (..., variable, ...)
825+
exists(AssignableDefinitions::TupleAssignmentDefinition tad, Ssa::ExplicitDefinition def |
821826
node2.(SsaDefinitionNode).getDefinition() = def and
822827
def.getADefinition() = tad and
823828
tad.getLeaf() = item and
824-
hasNodePath(x, node1, any(Node n | n.asExpr() = tad.getAssignment()))
825-
) and
826-
c.(FieldContent).getField() =
827-
node1.asExpr().getType().(TupleType).getElement(i).getUnboundDeclaration()
829+
hasNodePath(x, node1, node2)
830+
)
828831
)
829832
)
830833
or
@@ -1770,11 +1773,6 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
17701773
e1 = e2.(TupleExpr).getAnArgument() and
17711774
scope = e2 and
17721775
isSuccessor = true
1773-
or
1774-
exactScope = false and
1775-
e1.(TupleExpr).getParent*() = e2.(AssignExpr).getLValue() and
1776-
scope = e2 and
1777-
isSuccessor = true
17781776
}
17791777

17801778
override predicate candidateDef(
@@ -1796,6 +1794,14 @@ private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration
17961794
scope = fs.getVariableDeclExpr() and
17971795
exactScope = false
17981796
)
1797+
or
1798+
scope =
1799+
any(AssignExpr ae |
1800+
ae = defTo.(AssignableDefinitions::TupleAssignmentDefinition).getAssignment() and
1801+
e = ae.getLValue().getAChildExpr*().(TupleExpr) and
1802+
exactScope = false and
1803+
isSuccessor = true
1804+
)
17991805
}
18001806
}
18011807

‎csharp/ql/src/semmle/code/csharp/exprs/Expr.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1035,7 +1035,13 @@ class TupleExpr extends Expr, @tuple_expr {
10351035
Expr getAnArgument() { result = getArgument(_) }
10361036

10371037
/** Holds if this tuple is a read access. */
1038-
predicate isReadAccess() { not this = getAnAssignOrForeachChild() }
1038+
deprecated predicate isReadAccess() { not this = getAnAssignOrForeachChild() }
1039+
1040+
/** Holds if this expression is a tuple construction. */
1041+
predicate isConstruction() {
1042+
not this = getAnAssignOrForeachChild() and
1043+
not this instanceof PatternExpr
1044+
}
10391045

10401046
override string getAPrimaryQlClass() { result = "TupleExpr" }
10411047
}

‎csharp/ql/test/library-tests/csharp7/TupleAccess.expected

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,45 @@
1-
| CSharp7.cs:66:16:66:27 | (..., ...) | read |
2-
| CSharp7.cs:71:9:71:22 | (..., ...) | write |
3-
| CSharp7.cs:73:9:73:14 | (..., ...) | write |
4-
| CSharp7.cs:75:9:75:23 | (..., ...) | write |
5-
| CSharp7.cs:75:27:75:35 | (..., ...) | read |
6-
| CSharp7.cs:76:9:76:14 | (..., ...) | write |
7-
| CSharp7.cs:76:18:76:23 | (..., ...) | write |
8-
| CSharp7.cs:76:27:76:32 | (..., ...) | read |
9-
| CSharp7.cs:77:9:77:31 | (..., ...) | write |
10-
| CSharp7.cs:77:17:77:30 | (..., ...) | write |
11-
| CSharp7.cs:77:35:77:40 | (..., ...) | read |
12-
| CSharp7.cs:78:9:78:19 | (..., ...) | write |
13-
| CSharp7.cs:78:13:78:18 | (..., ...) | write |
14-
| CSharp7.cs:78:23:78:33 | (..., ...) | read |
15-
| CSharp7.cs:78:27:78:32 | (..., ...) | read |
16-
| CSharp7.cs:79:9:79:18 | (..., ...) | write |
17-
| CSharp7.cs:79:22:79:28 | (..., ...) | read |
18-
| CSharp7.cs:84:16:84:24 | (..., ...) | read |
19-
| CSharp7.cs:89:18:89:34 | (..., ...) | read |
20-
| CSharp7.cs:90:9:90:24 | (..., ...) | write |
21-
| CSharp7.cs:97:18:97:38 | (..., ...) | read |
22-
| CSharp7.cs:98:18:98:43 | (..., ...) | read |
23-
| CSharp7.cs:98:22:98:42 | (..., ...) | read |
24-
| CSharp7.cs:103:18:103:42 | (..., ...) | read |
25-
| CSharp7.cs:104:18:104:47 | (..., ...) | read |
26-
| CSharp7.cs:104:22:104:46 | (..., ...) | read |
27-
| CSharp7.cs:109:9:109:24 | (..., ...) | write |
28-
| CSharp7.cs:109:28:109:46 | (..., ...) | read |
29-
| CSharp7.cs:109:40:109:45 | (..., ...) | read |
30-
| CSharp7.cs:112:9:112:22 | (..., ...) | write |
31-
| CSharp7.cs:112:14:112:21 | (..., ...) | write |
32-
| CSharp7.cs:112:26:112:33 | (..., ...) | read |
33-
| CSharp7.cs:114:9:114:34 | (..., ...) | write |
34-
| CSharp7.cs:114:18:114:33 | (..., ...) | write |
35-
| CSharp7.cs:114:38:114:45 | (..., ...) | write |
36-
| CSharp7.cs:114:49:114:67 | (..., ...) | read |
37-
| CSharp7.cs:114:61:114:66 | (..., ...) | read |
38-
| CSharp7.cs:218:16:218:23 | (..., ...) | read |
39-
| CSharp7.cs:224:9:224:14 | (..., ...) | write |
40-
| CSharp7.cs:225:9:225:18 | (..., ...) | write |
41-
| CSharp7.cs:226:9:226:18 | (..., ...) | write |
42-
| CSharp7.cs:285:40:285:61 | (..., ...) | read |
43-
| CSharp7.cs:287:18:287:34 | (..., ...) | write |
44-
| CSharp7.cs:289:18:289:31 | (..., ...) | write |
45-
| CSharp7.cs:291:18:291:27 | (..., ...) | write |
1+
| CSharp7.cs:66:16:66:27 | (..., ...) | construct |
2+
| CSharp7.cs:71:9:71:22 | (..., ...) | deconstruct |
3+
| CSharp7.cs:73:9:73:14 | (..., ...) | deconstruct |
4+
| CSharp7.cs:75:9:75:23 | (..., ...) | deconstruct |
5+
| CSharp7.cs:75:27:75:35 | (..., ...) | construct |
6+
| CSharp7.cs:76:9:76:14 | (..., ...) | deconstruct |
7+
| CSharp7.cs:76:18:76:23 | (..., ...) | deconstruct |
8+
| CSharp7.cs:76:27:76:32 | (..., ...) | construct |
9+
| CSharp7.cs:77:9:77:31 | (..., ...) | deconstruct |
10+
| CSharp7.cs:77:17:77:30 | (..., ...) | deconstruct |
11+
| CSharp7.cs:77:35:77:40 | (..., ...) | construct |
12+
| CSharp7.cs:78:9:78:19 | (..., ...) | deconstruct |
13+
| CSharp7.cs:78:13:78:18 | (..., ...) | deconstruct |
14+
| CSharp7.cs:78:23:78:33 | (..., ...) | construct |
15+
| CSharp7.cs:78:27:78:32 | (..., ...) | construct |
16+
| CSharp7.cs:79:9:79:18 | (..., ...) | deconstruct |
17+
| CSharp7.cs:79:22:79:28 | (..., ...) | construct |
18+
| CSharp7.cs:84:16:84:24 | (..., ...) | construct |
19+
| CSharp7.cs:89:18:89:34 | (..., ...) | construct |
20+
| CSharp7.cs:90:9:90:24 | (..., ...) | deconstruct |
21+
| CSharp7.cs:97:18:97:38 | (..., ...) | construct |
22+
| CSharp7.cs:98:18:98:43 | (..., ...) | construct |
23+
| CSharp7.cs:98:22:98:42 | (..., ...) | construct |
24+
| CSharp7.cs:103:18:103:42 | (..., ...) | construct |
25+
| CSharp7.cs:104:18:104:47 | (..., ...) | construct |
26+
| CSharp7.cs:104:22:104:46 | (..., ...) | construct |
27+
| CSharp7.cs:109:9:109:24 | (..., ...) | deconstruct |
28+
| CSharp7.cs:109:28:109:46 | (..., ...) | construct |
29+
| CSharp7.cs:109:40:109:45 | (..., ...) | construct |
30+
| CSharp7.cs:112:9:112:22 | (..., ...) | deconstruct |
31+
| CSharp7.cs:112:14:112:21 | (..., ...) | deconstruct |
32+
| CSharp7.cs:112:26:112:33 | (..., ...) | construct |
33+
| CSharp7.cs:114:9:114:34 | (..., ...) | deconstruct |
34+
| CSharp7.cs:114:18:114:33 | (..., ...) | deconstruct |
35+
| CSharp7.cs:114:38:114:45 | (..., ...) | deconstruct |
36+
| CSharp7.cs:114:49:114:67 | (..., ...) | construct |
37+
| CSharp7.cs:114:61:114:66 | (..., ...) | construct |
38+
| CSharp7.cs:218:16:218:23 | (..., ...) | construct |
39+
| CSharp7.cs:224:9:224:14 | (..., ...) | deconstruct |
40+
| CSharp7.cs:225:9:225:18 | (..., ...) | deconstruct |
41+
| CSharp7.cs:226:9:226:18 | (..., ...) | deconstruct |
42+
| CSharp7.cs:285:40:285:61 | (..., ...) | construct |
43+
| CSharp7.cs:287:18:287:34 | (..., ...) | deconstruct |
44+
| CSharp7.cs:289:18:289:31 | (..., ...) | deconstruct |
45+
| CSharp7.cs:291:18:291:27 | (..., ...) | deconstruct |

‎csharp/ql/test/library-tests/csharp7/TupleAccess.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import csharp
22

33
from TupleExpr e, string access
4-
where if e.isReadAccess() then access = "read" else access = "write"
4+
where if e.isConstruction() then access = "construct" else access = "deconstruct"
55
select e, access

‎csharp/ql/test/library-tests/dataflow/tuples/Tuples.expected

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ edges
44
| Tuples.cs:7:17:7:56 | (..., ...) [Item1] : String | Tuples.cs:18:9:18:22 | (..., ...) [Item1] : String |
55
| Tuples.cs:7:17:7:56 | (..., ...) [Item1] : String | Tuples.cs:23:14:23:14 | access to local variable x [Item1] : String |
66
| Tuples.cs:7:17:7:56 | (..., ...) [Item1] : String | Tuples.cs:24:14:24:14 | access to local variable x [Item1] : String |
7-
| Tuples.cs:7:17:7:56 | (..., ...) [Item2, Item2] : String | Tuples.cs:7:37:7:55 | (..., ...) [Item2] : String |
87
| Tuples.cs:7:17:7:56 | (..., ...) [Item2, Item2] : String | Tuples.cs:8:9:8:23 | (..., ...) [Item2, Item2] : String |
98
| Tuples.cs:7:17:7:56 | (..., ...) [Item2, Item2] : String | Tuples.cs:13:9:13:19 | (..., ...) [Item2, Item2] : String |
109
| Tuples.cs:7:17:7:56 | (..., ...) [Item2, Item2] : String | Tuples.cs:18:9:18:22 | (..., ...) [Item2, Item2] : String |
@@ -14,15 +13,13 @@ edges
1413
| Tuples.cs:7:41:7:54 | "taint source" : String | Tuples.cs:7:37:7:55 | (..., ...) [Item2] : String |
1514
| Tuples.cs:8:9:8:23 | (..., ...) [Item1] : String | Tuples.cs:8:9:8:27 | SSA def(a) : String |
1615
| Tuples.cs:8:9:8:23 | (..., ...) [Item2, Item2] : String | Tuples.cs:8:9:8:23 | (..., ...) [Item2] : String |
17-
| Tuples.cs:8:9:8:23 | (..., ...) [Item2] : String | Tuples.cs:8:9:8:23 | (..., ...) [Item2, Item2] : String |
1816
| Tuples.cs:8:9:8:23 | (..., ...) [Item2] : String | Tuples.cs:8:9:8:27 | SSA def(c) : String |
1917
| Tuples.cs:8:9:8:27 | SSA def(a) : String | Tuples.cs:9:14:9:14 | access to local variable a |
2018
| Tuples.cs:8:9:8:27 | SSA def(c) : String | Tuples.cs:11:14:11:14 | access to local variable c |
2119
| Tuples.cs:13:9:13:19 | (..., ...) [Item1] : String | Tuples.cs:13:9:13:23 | SSA def(a) : String |
2220
| Tuples.cs:13:9:13:19 | (..., ...) [Item2, Item2] : String | Tuples.cs:13:13:13:18 | (..., ...) [Item2] : String |
2321
| Tuples.cs:13:9:13:23 | SSA def(a) : String | Tuples.cs:14:14:14:14 | access to local variable a |
2422
| Tuples.cs:13:9:13:23 | SSA def(c) : String | Tuples.cs:16:14:16:14 | access to local variable c |
25-
| Tuples.cs:13:13:13:18 | (..., ...) [Item2] : String | Tuples.cs:13:9:13:19 | (..., ...) [Item2, Item2] : String |
2623
| Tuples.cs:13:13:13:18 | (..., ...) [Item2] : String | Tuples.cs:13:9:13:23 | SSA def(c) : String |
2724
| Tuples.cs:18:9:18:22 | (..., ...) [Item1] : String | Tuples.cs:18:9:18:26 | SSA def(p) : String |
2825
| Tuples.cs:18:9:18:22 | (..., ...) [Item2, Item2] : String | Tuples.cs:18:9:18:26 | SSA def(q) [Item2] : String |

0 commit comments

Comments
 (0)