Skip to content

Commit a1fe8a5

Browse files
authored
python: handle not in BarrierGuard
in the program ```python if not is_safe(path): return ``` the last node in the `ConditionBlock` is `not is_safe(path)`, so it would never match "a call to is_safe". Thus, guards inside `not` would not be part of `GuardNode` (nor `BarrierGuard`). Now they can.
1 parent 882000a commit a1fe8a5

File tree

4 files changed

+32
-10
lines changed

4 files changed

+32
-10
lines changed

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -527,16 +527,32 @@ class StarPatternElementNode extends Node, TStarPatternElementNode {
527527
override Location getLocation() { result = consumer.getLocation() }
528528
}
529529

530+
ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) {
531+
result = conditionBlock.getLastNode() and
532+
flipped = false
533+
or
534+
exists(UnaryExprNode notNode |
535+
result = notNode.getOperand() and
536+
notNode.getNode().getOp() instanceof Not
537+
|
538+
notNode = guardNode(conditionBlock, flipped.booleanNot())
539+
)
540+
}
541+
530542
/**
531543
* A node that controls whether other nodes are evaluated.
532544
*/
533545
class GuardNode extends ControlFlowNode {
534546
ConditionBlock conditionBlock;
547+
boolean flipped;
535548

536-
GuardNode() { this = conditionBlock.getLastNode() }
549+
GuardNode() { this = guardNode(conditionBlock, flipped) }
537550

538551
/** Holds if this guard controls block `b` upon evaluating to `branch`. */
539-
predicate controlsBlock(BasicBlock b, boolean branch) { conditionBlock.controls(b, branch) }
552+
predicate controlsBlock(BasicBlock b, boolean branch) {
553+
branch in [true, false] and
554+
conditionBlock.controls(b, branch.booleanXor(flipped))
555+
}
540556
}
541557

542558
/**

python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/test_string_const_compare.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def test_non_eq2():
5050
if not ts == "safe":
5151
ensure_tainted(ts) # $ tainted
5252
else:
53-
ensure_not_tainted(ts) # $ SPURIOUS: tainted
53+
ensure_not_tainted(ts)
5454

5555

5656
def test_in_list():
@@ -157,7 +157,7 @@ def test_not_in2():
157157
if not ts in ["safe", "also_safe"]:
158158
ensure_tainted(ts) # $ tainted
159159
else:
160-
ensure_not_tainted(ts) # $ SPURIOUS: tainted
160+
ensure_not_tainted(ts)
161161

162162

163163
def is_safe(x):

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,19 @@ isSanitizer
77
| TestTaintTrackingConfiguration | test.py:52:28:52:28 | ControlFlowNode for s |
88
| TestTaintTrackingConfiguration | test.py:66:10:66:29 | ControlFlowNode for emulated_escaping() |
99
| TestTaintTrackingConfiguration | test_logical.py:33:28:33:28 | ControlFlowNode for s |
10+
| TestTaintTrackingConfiguration | test_logical.py:40:28:40:28 | ControlFlowNode for s |
1011
| TestTaintTrackingConfiguration | test_logical.py:48:28:48:28 | ControlFlowNode for s |
1112
| TestTaintTrackingConfiguration | test_logical.py:53:28:53:28 | ControlFlowNode for s |
1213
| TestTaintTrackingConfiguration | test_logical.py:92:28:92:28 | ControlFlowNode for s |
1314
| TestTaintTrackingConfiguration | test_logical.py:103:28:103:28 | ControlFlowNode for s |
15+
| TestTaintTrackingConfiguration | test_logical.py:111:28:111:28 | ControlFlowNode for s |
16+
| TestTaintTrackingConfiguration | test_logical.py:130:28:130:28 | ControlFlowNode for s |
17+
| TestTaintTrackingConfiguration | test_logical.py:137:28:137:28 | ControlFlowNode for s |
1418
| TestTaintTrackingConfiguration | test_logical.py:148:28:148:28 | ControlFlowNode for s |
1519
| TestTaintTrackingConfiguration | test_logical.py:151:28:151:28 | ControlFlowNode for s |
1620
| TestTaintTrackingConfiguration | test_logical.py:158:28:158:28 | ControlFlowNode for s |
21+
| TestTaintTrackingConfiguration | test_logical.py:167:24:167:24 | ControlFlowNode for s |
1722
| TestTaintTrackingConfiguration | test_logical.py:176:24:176:24 | ControlFlowNode for s |
23+
| TestTaintTrackingConfiguration | test_logical.py:185:24:185:24 | ControlFlowNode for s |
1824
| TestTaintTrackingConfiguration | test_logical.py:193:24:193:24 | ControlFlowNode for s |
1925
| TestTaintTrackingConfiguration | test_reference.py:31:28:31:28 | ControlFlowNode for s |

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/test_logical.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ def test_basic():
3737
if not is_safe(s):
3838
ensure_tainted(s) # $ tainted
3939
else:
40-
ensure_not_tainted(s) # $ SPURIOUS: tainted
40+
ensure_not_tainted(s)
4141

4242

4343
def test_if_in_depth():
@@ -108,7 +108,7 @@ def test_and():
108108
ensure_tainted(s) # $ tainted
109109
else:
110110
# cannot be tainted
111-
ensure_not_tainted(s) # $ SPURIOUS: tainted
111+
ensure_not_tainted(s)
112112

113113

114114
def test_tricky():
@@ -127,14 +127,14 @@ def test_nesting_not():
127127
s = TAINTED_STRING
128128

129129
if not(not(is_safe(s))):
130-
ensure_not_tainted(s) # $ SPURIOUS: tainted
130+
ensure_not_tainted(s)
131131
else:
132132
ensure_tainted(s) # $ tainted
133133

134134
if not(not(not(is_safe(s)))):
135135
ensure_tainted(s) # $ tainted
136136
else:
137-
ensure_not_tainted(s) # $ SPURIOUS: tainted
137+
ensure_not_tainted(s)
138138

139139

140140
# Adding `and True` makes the sanitizer trigger when it would otherwise not. See output in
@@ -164,7 +164,7 @@ def test_with_return():
164164
if not is_safe(s):
165165
return
166166

167-
ensure_not_tainted(s) # $ SPURIOUS: tainted
167+
ensure_not_tainted(s)
168168

169169

170170
def test_with_return_neg():
@@ -182,7 +182,7 @@ def test_with_exception():
182182
if not is_safe(s):
183183
raise Exception("unsafe")
184184

185-
ensure_not_tainted(s) # $ SPURIOUS: tainted
185+
ensure_not_tainted(s)
186186

187187
def test_with_exception_neg():
188188
s = TAINTED_STRING

0 commit comments

Comments
 (0)