Skip to content

Commit 7351e82

Browse files
committed
python: handle guards compared to boolean literals
1 parent 8488039 commit 7351e82

File tree

3 files changed

+45
-9
lines changed

3 files changed

+45
-9
lines changed

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,14 +595,42 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) {
595595
result = conditionBlock.getLastNode() and
596596
flipped = false
597597
or
598-
// Recursive case: if a guard node is a `not`-expression,
598+
// Recursive cases:
599+
// if a guard node is a `not`-expression,
599600
// the operand is also a guard node, but with inverted polarity.
600601
exists(UnaryExprNode notNode |
601602
result = notNode.getOperand() and
602603
notNode.getNode().getOp() instanceof Not
603604
|
604605
notNode = guardNode(conditionBlock, flipped.booleanNot())
605606
)
607+
or
608+
// if a guard node is compared to a boolean literal,
609+
// the other operand is also a guard node,
610+
// but with polarity depending on the literal (and on the comparison).
611+
exists(CompareNode cmpNode, Cmpop op, ControlFlowNode b, boolean bool |
612+
(
613+
cmpNode.operands(result, op, b) or
614+
cmpNode.operands(b, op, result)
615+
) and
616+
not result.getNode() instanceof BooleanLiteral and
617+
(
618+
// comparing to the boolean
619+
(op instanceof Eq or op instanceof Is) and
620+
// `bool` is the value being compared against, here the value of `b`
621+
b.getNode().(BooleanLiteral).booleanValue() = bool
622+
or
623+
// comparing to the negation of the boolean
624+
(op instanceof NotEq or op instanceof IsNot) and
625+
// again, `bool` is the value being compared against, but here it is the value of `not b`
626+
b.getNode().(BooleanLiteral).booleanValue() = bool.booleanNot()
627+
)
628+
|
629+
// if `bool` is true, we should preserve `flipped`, otherwise we should flip it
630+
// `flipped xor (not bool)` achieves that.
631+
flipped in [true, false] and
632+
cmpNode = guardNode(conditionBlock, flipped.booleanXor(bool.booleanNot()))
633+
)
606634
}
607635

608636
/**

python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/InlineTaintTest.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,12 @@ isSanitizer
2222
| test_logical.py:176:24:176:24 | ControlFlowNode for s |
2323
| test_logical.py:185:24:185:24 | ControlFlowNode for s |
2424
| test_logical.py:193:24:193:24 | ControlFlowNode for s |
25+
| test_logical.py:199:28:199:28 | ControlFlowNode for s |
26+
| test_logical.py:206:28:206:28 | ControlFlowNode for s |
27+
| test_logical.py:211:28:211:28 | ControlFlowNode for s |
28+
| test_logical.py:214:28:214:28 | ControlFlowNode for s |
29+
| test_logical.py:219:28:219:28 | ControlFlowNode for s |
30+
| test_logical.py:226:28:226:28 | ControlFlowNode for s |
31+
| test_logical.py:231:28:231:28 | ControlFlowNode for s |
32+
| test_logical.py:234:28:234:28 | ControlFlowNode for s |
2533
| test_reference.py:31:28:31:28 | ControlFlowNode for s |

python/ql/test/library-tests/dataflow/tainttracking/customSanitizer/test_logical.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,42 +196,42 @@ def test_comparison_with_bool():
196196
s = TAINTED_STRING
197197

198198
if is_safe(s) == True:
199-
ensure_not_tainted(s) # $ SPURIOUS: tainted
199+
ensure_not_tainted(s)
200200
else:
201201
ensure_tainted(s) # $ tainted
202202

203203
if is_safe(s) == False:
204204
ensure_tainted(s) # $ tainted
205205
else:
206-
ensure_not_tainted(s) # $ SPURIOUS: tainted
206+
ensure_not_tainted(s)
207207

208208
if is_safe(s) != True:
209209
ensure_tainted(s) # $ tainted
210210
else:
211-
ensure_not_tainted(s) # $ SPURIOUS: tainted
211+
ensure_not_tainted(s)
212212

213213
if is_safe(s) != False:
214-
ensure_not_tainted(s) # $ SPURIOUS: tainted
214+
ensure_not_tainted(s)
215215
else:
216216
ensure_tainted(s) # $ tainted
217217

218218
if is_safe(s) is True:
219-
ensure_not_tainted(s) # $ SPURIOUS: tainted
219+
ensure_not_tainted(s)
220220
else:
221221
ensure_tainted(s) # $ tainted
222222

223223
if is_safe(s) is False:
224224
ensure_tainted(s) # $ tainted
225225
else:
226-
ensure_not_tainted(s) # $ SPURIOUS: tainted
226+
ensure_not_tainted(s)
227227

228228
if is_safe(s) is not True:
229229
ensure_tainted(s) # $ tainted
230230
else:
231-
ensure_not_tainted(s) # $ SPURIOUS: tainted
231+
ensure_not_tainted(s)
232232

233233
if is_safe(s) is not False:
234-
ensure_not_tainted(s) # $ SPURIOUS: tainted
234+
ensure_not_tainted(s)
235235
else:
236236
ensure_tainted(s) # $ tainted
237237

0 commit comments

Comments
 (0)