From 32c4db4e50f4ff3b51057dd53bb053279b6eef45 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 20 Feb 2026 14:53:39 +0000 Subject: [PATCH 1/2] Python: Port NonIteratorInForLoop.ql Uses the `DuckTyping` module to approximate whether something is likely to be an iterator or not. We lose one test result due to the fact that we don't know what to do about `for ... in 1` (because `1` is an instance of a built-in). I'm going to defer addressing this until we get some modelling of built-in types. --- .../ql/src/Statements/NonIteratorInForLoop.ql | 21 +++++++++---------- .../iter/NonIteratorInForLoop.expected | 3 +-- .../general/NonIteratorInForLoop.expected | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/python/ql/src/Statements/NonIteratorInForLoop.ql b/python/ql/src/Statements/NonIteratorInForLoop.ql index f8e6e51b55ff..44dfa967bff9 100644 --- a/python/ql/src/Statements/NonIteratorInForLoop.ql +++ b/python/ql/src/Statements/NonIteratorInForLoop.ql @@ -12,16 +12,15 @@ */ import python -private import LegacyPointsTo +private import semmle.python.dataflow.new.internal.DataFlowDispatch -from For loop, ControlFlowNodeWithPointsTo iter, Value v, ClassValue t, ControlFlowNode origin +from For loop, Expr iter, Class cls where - loop.getIter().getAFlowNode() = iter and - iter.pointsTo(_, v, origin) and - v.getClass() = t and - not t.isIterable() and - not t.failedInference(_) and - not v = Value::named("None") and - not t.isDescriptorType() -select loop, "This for-loop may attempt to iterate over a $@ of class $@.", origin, - "non-iterable instance", t, t.getName() + iter = loop.getIter() and + classInstanceTracker(cls).asExpr() = iter and + not DuckTyping::isIterable(cls) and + not DuckTyping::isDescriptor(cls) and + not (loop.isAsync() and DuckTyping::hasMethod(cls, "__aiter__")) and + not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls)) +select loop, "This for-loop may attempt to iterate over a $@ of class $@.", iter, + "non-iterable instance", cls, cls.getName() diff --git a/python/ql/test/3/query-tests/Statements/iter/NonIteratorInForLoop.expected b/python/ql/test/3/query-tests/Statements/iter/NonIteratorInForLoop.expected index c59db3b2b657..396382d0b9f9 100644 --- a/python/ql/test/3/query-tests/Statements/iter/NonIteratorInForLoop.expected +++ b/python/ql/test/3/query-tests/Statements/iter/NonIteratorInForLoop.expected @@ -1,2 +1 @@ -| async_iterator.py:26:11:26:34 | For | This for-loop may attempt to iterate over a $@ of class $@. | async_iterator.py:26:20:26:33 | ControlFlowNode for MissingAiter() | non-iterable instance | async_iterator.py:13:1:13:19 | class MissingAiter | MissingAiter | -| statements_test.py:34:5:34:19 | For | This for-loop may attempt to iterate over a $@ of class $@. | statements_test.py:34:18:34:18 | ControlFlowNode for IntegerLiteral | non-iterable instance | file://:0:0:0:0 | builtin-class int | int | +| async_iterator.py:26:11:26:34 | For | This for-loop may attempt to iterate over a $@ of class $@. | async_iterator.py:26:20:26:33 | MissingAiter() | non-iterable instance | async_iterator.py:13:1:13:19 | Class MissingAiter | MissingAiter | diff --git a/python/ql/test/query-tests/Statements/general/NonIteratorInForLoop.expected b/python/ql/test/query-tests/Statements/general/NonIteratorInForLoop.expected index 4c79685061f5..d11b87191ba6 100644 --- a/python/ql/test/query-tests/Statements/general/NonIteratorInForLoop.expected +++ b/python/ql/test/query-tests/Statements/general/NonIteratorInForLoop.expected @@ -1 +1 @@ -| test.py:50:1:50:23 | For | This for-loop may attempt to iterate over a $@ of class $@. | test.py:50:10:50:22 | ControlFlowNode for NonIterator() | non-iterable instance | test.py:45:1:45:26 | class NonIterator | NonIterator | +| test.py:50:1:50:23 | For | This for-loop may attempt to iterate over a $@ of class $@. | test.py:50:10:50:22 | NonIterator() | non-iterable instance | test.py:45:1:45:26 | Class NonIterator | NonIterator | From 8fdf5c93224d1b0b4274b97cd9eaa20c0f4fac61 Mon Sep 17 00:00:00 2001 From: Taus Date: Mon, 16 Mar 2026 14:33:59 +0000 Subject: [PATCH 2/2] Python: Exclude iterators guarded by `isinstance` checks A common pattern is to check `isinstance(it, (list, tuple))` before proceeding with the iteration. --- .../ql/src/Statements/NonIteratorInForLoop.ql | 43 ++++++++++++++++--- .../query-tests/Statements/general/test.py | 21 +++++++++ 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/python/ql/src/Statements/NonIteratorInForLoop.ql b/python/ql/src/Statements/NonIteratorInForLoop.ql index 44dfa967bff9..2c04c8d97ee5 100644 --- a/python/ql/src/Statements/NonIteratorInForLoop.ql +++ b/python/ql/src/Statements/NonIteratorInForLoop.ql @@ -13,14 +13,47 @@ import python private import semmle.python.dataflow.new.internal.DataFlowDispatch +private import semmle.python.ApiGraphs -from For loop, Expr iter, Class cls +/** + * Holds if `cls_arg` references a known iterable builtin type, either directly + * (e.g. `list`) or as an element of a tuple (e.g. `(list, tuple)`). + */ +private predicate isIterableTypeArg(DataFlow::Node cls_arg) { + cls_arg = + API::builtin([ + "list", "tuple", "set", "frozenset", "dict", "str", "bytes", "bytearray", "range", + "memoryview" + ]).getAValueReachableFromSource() + or + isIterableTypeArg(DataFlow::exprNode(cls_arg.asExpr().(Tuple).getAnElt())) +} + +/** + * Holds if `iter` is guarded by an `isinstance` check that tests for + * an iterable type (e.g. `list`, `tuple`, `set`, `dict`). + */ +predicate guardedByIsinstanceIterable(DataFlow::Node iter) { + exists( + DataFlow::GuardNode guard, DataFlow::CallCfgNode isinstance_call, DataFlow::LocalSourceNode src + | + isinstance_call = API::builtin("isinstance").getACall() and + src.flowsTo(isinstance_call.getArg(0)) and + src.flowsTo(iter) and + isIterableTypeArg(isinstance_call.getArg(1)) and + guard = isinstance_call.asCfgNode() and + guard.controlsBlock(iter.asCfgNode().getBasicBlock(), true) + ) +} + +from For loop, DataFlow::Node iter, Class cls where - iter = loop.getIter() and - classInstanceTracker(cls).asExpr() = iter and + iter.asExpr() = loop.getIter() and + iter = classInstanceTracker(cls) and not DuckTyping::isIterable(cls) and not DuckTyping::isDescriptor(cls) and not (loop.isAsync() and DuckTyping::hasMethod(cls, "__aiter__")) and - not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls)) -select loop, "This for-loop may attempt to iterate over a $@ of class $@.", iter, + not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls)) and + not guardedByIsinstanceIterable(iter) +select loop, "This for-loop may attempt to iterate over a $@ of class $@.", iter.asExpr(), "non-iterable instance", cls, cls.getName() diff --git a/python/ql/test/query-tests/Statements/general/test.py b/python/ql/test/query-tests/Statements/general/test.py index eee63fa89e88..7a22fe292321 100644 --- a/python/ql/test/query-tests/Statements/general/test.py +++ b/python/ql/test/query-tests/Statements/general/test.py @@ -174,3 +174,24 @@ def assert_ok(seq): # False positive. ODASA-8042. Fixed in PR #2401. class false_positive: e = (x for x in []) + +# isinstance guard should suppress non-iterable warning +def guarded_iteration(x): + ni = NonIterator() + if isinstance(ni, (list, tuple)): + for item in ni: + pass + +def guarded_iteration_single(x): + ni = NonIterator() + if isinstance(ni, list): + for item in ni: + pass + +# Negated isinstance guard: early return when NOT iterable +def guarded_iteration_negated(x): + ni = NonIterator() + if not isinstance(ni, list): + return + for item in ni: # OK: guarded by negated isinstance + early return + pass