From 820d94098fb9c258e827bcb12822e47b1cac010e Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Thu, 1 Dec 2022 09:56:51 +0100 Subject: [PATCH] python: port `py/comparison-using-is` see triage [here](https://github.com/github/codeql-python-team/issues/628#issuecomment-1328933001) - no longer try to interpret the class of operands - simply alert in clear bad cases of uninterned literals - surprisingly(?), all tests still pass --- .../Expressions/IncorrectComparisonUsingIs.ql | 57 ++++++++++++++++--- 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/python/ql/src/Expressions/IncorrectComparisonUsingIs.ql b/python/ql/src/Expressions/IncorrectComparisonUsingIs.ql index 5352dc9c9fa..aab2ba6bfa1 100644 --- a/python/ql/src/Expressions/IncorrectComparisonUsingIs.ql +++ b/python/ql/src/Expressions/IncorrectComparisonUsingIs.ql @@ -11,16 +11,57 @@ */ import python -import IsComparisons -from Compare comp, Cmpop op, ClassValue c, string alt -where - invalid_portable_is_comparison(comp, op, c) and - not cpython_interned_constant(comp.getASubExpression()) and - ( - op instanceof Is and alt = "==" +/** Holds if the comparison `comp` uses `is` or `is not` (represented as `op`) to compare its `left` and `right` arguments. */ +predicate comparison_using_is(Compare comp, ControlFlowNode left, Cmpop op, ControlFlowNode right) { + exists(CompareNode fcomp | fcomp = comp.getAFlowNode() | + fcomp.operands(left, op, right) and + (op instanceof Is or op instanceof IsNot) + ) +} + +private predicate cpython_interned_value(Expr e) { + exists(string text | text = e.(StrConst).getText() | + text.length() = 0 or - op instanceof IsNot and alt = "!=" + text.length() = 1 and text.regexpMatch("[U+0000-U+00ff]") + ) + or + exists(int i | i = e.(IntegerLiteral).getN().toInt() | -5 <= i and i <= 256) + or + exists(Tuple t | t = e and not exists(t.getAnElt())) +} + +predicate uninterned_literal(Expr e) { + ( + e instanceof StrConst + or + e instanceof IntegerLiteral + or + e instanceof FloatLiteral + or + e instanceof Dict + or + e instanceof List + or + e instanceof Tuple + ) and + not cpython_interned_value(e) +} + +from Compare comp, Cmpop op, string alt +where + exists(ControlFlowNode left, ControlFlowNode right | + comparison_using_is(comp, left, op, right) and + ( + op instanceof Is and alt = "==" + or + op instanceof IsNot and alt = "!=" + ) + | + uninterned_literal(left.getNode()) + or + uninterned_literal(right.getNode()) ) select comp, "Values compared using '" + op.getSymbol() +