Skip to content

Commit 4fd3fc6

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

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
@@ -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/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)