Skip to content

Commit a762373

Browse files
yoffyoff
committed
Python: Implement simple barrier guard
The one found in the original test case
1 parent 49ae549 commit a762373

File tree

4 files changed

+143
-27
lines changed

4 files changed

+143
-27
lines changed

‎python/ql/src/semmle/python/functions/ModificationOfParameterWithDefault.qll

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,21 @@ module ModificationOfParameterWithDefault {
1919
* A data-flow configuration for detecting modifications of a parameters default value.
2020
*/
2121
class Configuration extends DataFlow::Configuration {
22-
Configuration() { this = "ModificationOfParameterWithDefault" }
22+
boolean nonEmpty;
2323

24-
override predicate isSource(DataFlow::Node source) { source instanceof Source }
24+
Configuration() {
25+
nonEmpty in [true, false] and
26+
this = "ModificationOfParameterWithDefault:" + nonEmpty.toString()
27+
}
28+
29+
override predicate isSource(DataFlow::Node source) { source.(Source).isNonEmpty() = nonEmpty }
2530

2631
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
2732

2833
override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
2934

3035
override predicate isBarrierGuard(DataFlow::BarrierGuard guard) {
31-
guard instanceof BarrierGuard
36+
guard.(BarrierGuard).blocksNonEmpty() = nonEmpty
3237
}
3338
}
3439
}

‎python/ql/src/semmle/python/functions/ModificationOfParameterWithDefaultCustomizations.qll

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ module ModificationOfParameterWithDefault {
1616
/**
1717
* A data flow source for detecting modifications of a parameters default value.
1818
*/
19-
abstract class Source extends DataFlow::Node { }
19+
abstract class Source extends DataFlow::Node {
20+
abstract boolean isNonEmpty();
21+
}
2022

2123
/**
2224
* A data flow sink for detecting modifications of a parameters default value.
@@ -31,7 +33,9 @@ module ModificationOfParameterWithDefault {
3133
/**
3234
* A sanitizer guard for detecting modifications of a parameters default value.
3335
*/
34-
abstract class BarrierGuard extends DataFlow::BarrierGuard { }
36+
abstract class BarrierGuard extends DataFlow::BarrierGuard {
37+
abstract boolean blocksNonEmpty();
38+
}
3539

3640
/** Gets the truthiness (non emptyness) of the default of `p` if that value is mutable */
3741
private boolean mutableDefaultValue(Parameter p) {
@@ -55,6 +59,8 @@ module ModificationOfParameterWithDefault {
5559
boolean nonEmpty;
5660

5761
MutableDefaultValue() { nonEmpty = mutableDefaultValue(this.asCfgNode().(NameNode).getNode()) }
62+
63+
override boolean isNonEmpty() { result = nonEmpty }
5864
}
5965

6066
/**
@@ -113,4 +119,47 @@ module ModificationOfParameterWithDefault {
113119
)
114120
}
115121
}
122+
123+
private class IdentityGuarded extends Expr {
124+
boolean inverted;
125+
126+
IdentityGuarded() {
127+
this = any(If i).getTest() and
128+
inverted = false
129+
or
130+
exists(IdentityGuarded ig, UnaryExpr notExp |
131+
notExp.getOp() instanceof Not and
132+
ig = notExp and
133+
notExp.getOperand() = this
134+
|
135+
inverted = ig.isInverted().booleanNot()
136+
)
137+
}
138+
139+
boolean isInverted() { result = inverted }
140+
}
141+
142+
class IdentityGuard extends BarrierGuard {
143+
ControlFlowNode checked_node;
144+
boolean safe_branch;
145+
boolean nonEmpty;
146+
147+
IdentityGuard() {
148+
nonEmpty in [true, false] and
149+
exists(IdentityGuarded ig |
150+
this.getNode() = ig and
151+
checked_node = this and
152+
// The raw guard is true if the value is non-empty
153+
// So we are safe either if we are looking for a non-empty value
154+
// or if we are looking for an empty value and the guard is inverted.
155+
safe_branch = ig.isInverted().booleanXor(nonEmpty)
156+
)
157+
}
158+
159+
override predicate checks(ControlFlowNode node, boolean branch) {
160+
node = checked_node and branch = safe_branch
161+
}
162+
163+
override boolean blocksNonEmpty() { result = nonEmpty }
164+
}
116165
}

‎python/ql/test/query-tests/Functions/ModificationOfParameterWithDefault/ModificationOfParameterWithDefault.expected

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,37 @@
11
edges
2+
| test.py:2:12:2:12 | ControlFlowNode for l | test.py:3:5:3:5 | ControlFlowNode for l |
3+
| test.py:7:11:7:11 | ControlFlowNode for l | test.py:8:5:8:5 | ControlFlowNode for l |
4+
| test.py:12:14:12:14 | ControlFlowNode for l | test.py:13:9:13:9 | ControlFlowNode for l |
25
| test.py:17:15:17:15 | ControlFlowNode for l | test.py:18:5:18:5 | ControlFlowNode for l |
36
| test.py:22:15:22:15 | ControlFlowNode for l | test.py:23:5:23:5 | ControlFlowNode for l |
47
| test.py:27:12:27:12 | ControlFlowNode for l | test.py:28:5:28:5 | ControlFlowNode for l |
58
| test.py:38:13:38:13 | ControlFlowNode for l | test.py:39:5:39:5 | ControlFlowNode for l |
69
| test.py:43:14:43:14 | ControlFlowNode for l | test.py:44:13:44:13 | ControlFlowNode for l |
710
| test.py:44:13:44:13 | ControlFlowNode for l | test.py:38:13:38:13 | ControlFlowNode for l |
811
| test.py:48:14:48:14 | ControlFlowNode for l | test.py:49:5:49:5 | ControlFlowNode for l |
12+
| test.py:53:10:53:10 | ControlFlowNode for d | test.py:54:5:54:5 | ControlFlowNode for d |
13+
| test.py:58:19:58:19 | ControlFlowNode for d | test.py:59:5:59:5 | ControlFlowNode for d |
14+
| test.py:63:28:63:28 | ControlFlowNode for d | test.py:64:5:64:5 | ControlFlowNode for d |
15+
| test.py:67:14:67:14 | ControlFlowNode for d | test.py:68:5:68:5 | ControlFlowNode for d |
16+
| test.py:72:19:72:19 | ControlFlowNode for d | test.py:73:14:73:14 | ControlFlowNode for d |
17+
| test.py:73:14:73:14 | ControlFlowNode for d | test.py:67:14:67:14 | ControlFlowNode for d |
918
| test.py:77:17:77:17 | ControlFlowNode for d | test.py:78:5:78:5 | ControlFlowNode for d |
1019
| test.py:82:26:82:26 | ControlFlowNode for d | test.py:83:5:83:5 | ControlFlowNode for d |
1120
| test.py:87:35:87:35 | ControlFlowNode for d | test.py:88:5:88:5 | ControlFlowNode for d |
1221
| test.py:91:21:91:21 | ControlFlowNode for d | test.py:92:5:92:5 | ControlFlowNode for d |
1322
| test.py:96:26:96:26 | ControlFlowNode for d | test.py:97:21:97:21 | ControlFlowNode for d |
1423
| test.py:97:21:97:21 | ControlFlowNode for d | test.py:91:21:91:21 | ControlFlowNode for d |
24+
| test.py:108:14:108:14 | ControlFlowNode for d | test.py:109:9:109:9 | ControlFlowNode for d |
1525
| test.py:113:20:113:20 | ControlFlowNode for d | test.py:115:5:115:5 | ControlFlowNode for d |
1626
| test.py:119:29:119:29 | ControlFlowNode for d | test.py:121:5:121:5 | ControlFlowNode for d |
1727
| test.py:125:15:125:15 | ControlFlowNode for l | test.py:127:9:127:9 | ControlFlowNode for l |
1828
nodes
29+
| test.py:2:12:2:12 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
30+
| test.py:3:5:3:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
31+
| test.py:7:11:7:11 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
32+
| test.py:8:5:8:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
33+
| test.py:12:14:12:14 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
34+
| test.py:13:9:13:9 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
1935
| test.py:17:15:17:15 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
2036
| test.py:18:5:18:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
2137
| test.py:22:15:22:15 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
@@ -28,6 +44,16 @@ nodes
2844
| test.py:44:13:44:13 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
2945
| test.py:48:14:48:14 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
3046
| test.py:49:5:49:5 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
47+
| test.py:53:10:53:10 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
48+
| test.py:54:5:54:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
49+
| test.py:58:19:58:19 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
50+
| test.py:59:5:59:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
51+
| test.py:63:28:63:28 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
52+
| test.py:64:5:64:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
53+
| test.py:67:14:67:14 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
54+
| test.py:68:5:68:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
55+
| test.py:72:19:72:19 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
56+
| test.py:73:14:73:14 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
3157
| test.py:77:17:77:17 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
3258
| test.py:78:5:78:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
3359
| test.py:82:26:82:26 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
@@ -38,22 +64,32 @@ nodes
3864
| test.py:92:5:92:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
3965
| test.py:96:26:96:26 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4066
| test.py:97:21:97:21 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
67+
| test.py:108:14:108:14 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
68+
| test.py:109:9:109:9 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4169
| test.py:113:20:113:20 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4270
| test.py:115:5:115:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4371
| test.py:119:29:119:29 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4472
| test.py:121:5:121:5 | ControlFlowNode for d | semmle.label | ControlFlowNode for d |
4573
| test.py:125:15:125:15 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
4674
| test.py:127:9:127:9 | ControlFlowNode for l | semmle.label | ControlFlowNode for l |
4775
#select
76+
| test.py:3:5:3:5 | ControlFlowNode for l | test.py:2:12:2:12 | ControlFlowNode for l | test.py:3:5:3:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:2:12:2:12 | ControlFlowNode for l | Default value |
77+
| test.py:8:5:8:5 | ControlFlowNode for l | test.py:7:11:7:11 | ControlFlowNode for l | test.py:8:5:8:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:7:11:7:11 | ControlFlowNode for l | Default value |
78+
| test.py:13:9:13:9 | ControlFlowNode for l | test.py:12:14:12:14 | ControlFlowNode for l | test.py:13:9:13:9 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:12:14:12:14 | ControlFlowNode for l | Default value |
4879
| test.py:18:5:18:5 | ControlFlowNode for l | test.py:17:15:17:15 | ControlFlowNode for l | test.py:18:5:18:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:17:15:17:15 | ControlFlowNode for l | Default value |
4980
| test.py:23:5:23:5 | ControlFlowNode for l | test.py:22:15:22:15 | ControlFlowNode for l | test.py:23:5:23:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:22:15:22:15 | ControlFlowNode for l | Default value |
5081
| test.py:28:5:28:5 | ControlFlowNode for l | test.py:27:12:27:12 | ControlFlowNode for l | test.py:28:5:28:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:27:12:27:12 | ControlFlowNode for l | Default value |
5182
| test.py:39:5:39:5 | ControlFlowNode for l | test.py:43:14:43:14 | ControlFlowNode for l | test.py:39:5:39:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:43:14:43:14 | ControlFlowNode for l | Default value |
5283
| test.py:49:5:49:5 | ControlFlowNode for l | test.py:48:14:48:14 | ControlFlowNode for l | test.py:49:5:49:5 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:48:14:48:14 | ControlFlowNode for l | Default value |
84+
| test.py:54:5:54:5 | ControlFlowNode for d | test.py:53:10:53:10 | ControlFlowNode for d | test.py:54:5:54:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:53:10:53:10 | ControlFlowNode for d | Default value |
85+
| test.py:59:5:59:5 | ControlFlowNode for d | test.py:58:19:58:19 | ControlFlowNode for d | test.py:59:5:59:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:58:19:58:19 | ControlFlowNode for d | Default value |
86+
| test.py:64:5:64:5 | ControlFlowNode for d | test.py:63:28:63:28 | ControlFlowNode for d | test.py:64:5:64:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:63:28:63:28 | ControlFlowNode for d | Default value |
87+
| test.py:68:5:68:5 | ControlFlowNode for d | test.py:72:19:72:19 | ControlFlowNode for d | test.py:68:5:68:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:72:19:72:19 | ControlFlowNode for d | Default value |
5388
| test.py:78:5:78:5 | ControlFlowNode for d | test.py:77:17:77:17 | ControlFlowNode for d | test.py:78:5:78:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:77:17:77:17 | ControlFlowNode for d | Default value |
5489
| test.py:83:5:83:5 | ControlFlowNode for d | test.py:82:26:82:26 | ControlFlowNode for d | test.py:83:5:83:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:82:26:82:26 | ControlFlowNode for d | Default value |
5590
| test.py:88:5:88:5 | ControlFlowNode for d | test.py:87:35:87:35 | ControlFlowNode for d | test.py:88:5:88:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:87:35:87:35 | ControlFlowNode for d | Default value |
5691
| test.py:92:5:92:5 | ControlFlowNode for d | test.py:96:26:96:26 | ControlFlowNode for d | test.py:92:5:92:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:96:26:96:26 | ControlFlowNode for d | Default value |
92+
| test.py:109:9:109:9 | ControlFlowNode for d | test.py:108:14:108:14 | ControlFlowNode for d | test.py:109:9:109:9 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:108:14:108:14 | ControlFlowNode for d | Default value |
5793
| test.py:115:5:115:5 | ControlFlowNode for d | test.py:113:20:113:20 | ControlFlowNode for d | test.py:115:5:115:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:113:20:113:20 | ControlFlowNode for d | Default value |
5894
| test.py:121:5:121:5 | ControlFlowNode for d | test.py:119:29:119:29 | ControlFlowNode for d | test.py:121:5:121:5 | ControlFlowNode for d | $@ flows to here and is mutated. | test.py:119:29:119:29 | ControlFlowNode for d | Default value |
5995
| test.py:127:9:127:9 | ControlFlowNode for l | test.py:125:15:125:15 | ControlFlowNode for l | test.py:127:9:127:9 | ControlFlowNode for l | $@ flows to here and is mutated. | test.py:125:15:125:15 | ControlFlowNode for l | Default value |

0 commit comments

Comments
 (0)