Skip to content

Commit 820d940

Browse files
committed
python: port py/comparison-using-is
see triage [here](github/codeql-python-team#628 (comment)) - no longer try to interpret the class of operands - simply alert in clear bad cases of uninterned literals - surprisingly(?), all tests still pass
1 parent 7c1bfdb commit 820d940

File tree

1 file changed

+48
-7
lines changed

1 file changed

+48
-7
lines changed

python/ql/src/Expressions/IncorrectComparisonUsingIs.ql

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,57 @@
1111
*/
1212

1313
import python
14-
import IsComparisons
1514

16-
from Compare comp, Cmpop op, ClassValue c, string alt
17-
where
18-
invalid_portable_is_comparison(comp, op, c) and
19-
not cpython_interned_constant(comp.getASubExpression()) and
15+
/** Holds if the comparison `comp` uses `is` or `is not` (represented as `op`) to compare its `left` and `right` arguments. */
16+
predicate comparison_using_is(Compare comp, ControlFlowNode left, Cmpop op, ControlFlowNode right) {
17+
exists(CompareNode fcomp | fcomp = comp.getAFlowNode() |
18+
fcomp.operands(left, op, right) and
19+
(op instanceof Is or op instanceof IsNot)
20+
)
21+
}
22+
23+
private predicate cpython_interned_value(Expr e) {
24+
exists(string text | text = e.(StrConst).getText() |
25+
text.length() = 0
26+
or
27+
text.length() = 1 and text.regexpMatch("[U+0000-U+00ff]")
28+
)
29+
or
30+
exists(int i | i = e.(IntegerLiteral).getN().toInt() | -5 <= i and i <= 256)
31+
or
32+
exists(Tuple t | t = e and not exists(t.getAnElt()))
33+
}
34+
35+
predicate uninterned_literal(Expr e) {
2036
(
21-
op instanceof Is and alt = "=="
37+
e instanceof StrConst
38+
or
39+
e instanceof IntegerLiteral
40+
or
41+
e instanceof FloatLiteral
42+
or
43+
e instanceof Dict
44+
or
45+
e instanceof List
46+
or
47+
e instanceof Tuple
48+
) and
49+
not cpython_interned_value(e)
50+
}
51+
52+
from Compare comp, Cmpop op, string alt
53+
where
54+
exists(ControlFlowNode left, ControlFlowNode right |
55+
comparison_using_is(comp, left, op, right) and
56+
(
57+
op instanceof Is and alt = "=="
58+
or
59+
op instanceof IsNot and alt = "!="
60+
)
61+
|
62+
uninterned_literal(left.getNode())
2263
or
23-
op instanceof IsNot and alt = "!="
64+
uninterned_literal(right.getNode())
2465
)
2566
select comp,
2667
"Values compared using '" + op.getSymbol() +

0 commit comments

Comments
 (0)