Skip to content

Commit f7188cb

Browse files
committed
python: handle guards compared to boolean literals
1 parent d2bab38 commit f7188cb

File tree

2 files changed

+37
-9
lines changed

2 files changed

+37
-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
@@ -553,14 +553,42 @@ ControlFlowNode guardNode(ConditionBlock conditionBlock, boolean flipped) {
553553
result = conditionBlock.getLastNode() and
554554
flipped = false
555555
or
556-
// Recursive case: if a guard node is a `not`-expression,
556+
// Recursive cases:
557+
// if a guard node is a `not`-expression,
557558
// the operand is also a guard node, but with inverted polarity.
558559
exists(UnaryExprNode notNode |
559560
result = notNode.getOperand() and
560561
notNode.getNode().getOp() instanceof Not
561562
|
562563
notNode = guardNode(conditionBlock, flipped.booleanNot())
563564
)
565+
or
566+
// if a guard node is compared to a boolean literal,
567+
// the other operand is also a guard node,
568+
// but with polarity depending on the literal (and on the comparison).
569+
exists(CompareNode cmpNode, Cmpop op, ControlFlowNode b, boolean bool |
570+
(
571+
cmpNode.operands(result, op, b) or
572+
cmpNode.operands(b, op, result)
573+
) and
574+
not result.getNode() instanceof BooleanLiteral and
575+
(
576+
// comparing to the boolean
577+
(op instanceof Eq or op instanceof Is) and
578+
// `bool` is the value being compared against, here the value of `b`
579+
b.getNode().(BooleanLiteral).booleanValue() = bool
580+
or
581+
// comparing to the negation of the boolean
582+
(op instanceof NotEq or op instanceof IsNot) and
583+
// again, `bool` is the value being compared against, but here it is the value of `not b`
584+
b.getNode().(BooleanLiteral).booleanValue() = bool.booleanNot()
585+
)
586+
|
587+
// if `bool` is true, we should preserve `flipped`, otherwise we should flip it
588+
// `flipped xor (not bool)` achieves that.
589+
flipped in [true, false] and
590+
cmpNode = guardNode(conditionBlock, flipped.booleanXor(bool.booleanNot()))
591+
)
564592
}
565593

566594
/**

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)