From 3e6e7295c2fdec18b6f20d963bb8b4270feb3c6a Mon Sep 17 00:00:00 2001 From: Taus Date: Mon, 16 Mar 2026 14:36:29 +0000 Subject: [PATCH] Python: Remove some FPs for ContainsNonContainer.ql First fix handles the case where there's interference from a class-based decorator on a function. In this case, _technically_ we have an instance of the decorator class, but in practice this decorator will (hopefully) forward all accesses to the thing it wraps. The second fix has to do with methods that are added dynamically using `setattr`. In this case, we cannot be sure that the relevant methods are actually missing. --- .../src/Expressions/ContainsNonContainer.ql | 23 +++++++++++ .../Expressions/general/expressions_test.py | 38 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/python/ql/src/Expressions/ContainsNonContainer.ql b/python/ql/src/Expressions/ContainsNonContainer.ql index a1b98902058..ad0a80bb1dd 100644 --- a/python/ql/src/Expressions/ContainsNonContainer.ql +++ b/python/ql/src/Expressions/ContainsNonContainer.ql @@ -21,12 +21,35 @@ predicate rhs_in_expr(Expr rhs, Compare cmp) { ) } +/** + * Holds if `origin` is the result of applying a class as a decorator to a function. + * Such decorator classes act as proxies, and the runtime value of the decorated + * attribute may be of a different type than the decorator class itself. + */ +predicate isDecoratorApplication(DataFlow::LocalSourceNode origin) { + exists(FunctionExpr fe | origin.asExpr() = fe.getADecoratorCall()) +} + +/** + * Holds if `cls` has methods dynamically added via `setattr`, so we cannot + * statically determine its full interface. + */ +predicate hasDynamicMethods(Class cls) { + exists(CallNode setattr_call | + setattr_call.getFunction().(NameNode).getId() = "setattr" and + setattr_call.getArg(0).(NameNode).getId() = cls.getName() and + setattr_call.getScope() = cls.getScope() + ) +} + from Compare cmp, DataFlow::LocalSourceNode origin, DataFlow::Node rhs, Class cls where origin = classInstanceTracker(cls) and origin.flowsTo(rhs) and not DuckTyping::isContainer(cls) and not DuckTyping::hasUnresolvedBase(getADirectSuperclass*(cls)) and + not isDecoratorApplication(origin) and + not hasDynamicMethods(cls) and rhs_in_expr(rhs.asExpr(), cmp) select cmp, "This test may raise an Exception as the $@ may be of non-container class $@.", origin, "target", cls, cls.getName() diff --git a/python/ql/test/query-tests/Expressions/general/expressions_test.py b/python/ql/test/query-tests/Expressions/general/expressions_test.py index 5e07b58e204..cf4c04e0a34 100644 --- a/python/ql/test/query-tests/Expressions/general/expressions_test.py +++ b/python/ql/test/query-tests/Expressions/general/expressions_test.py @@ -279,3 +279,41 @@ def useofapply(): def apply(f): pass apply(foo)([1]) + +# Class used as a decorator: the runtime value at attribute access is the +# function's return value, not the decorator class instance. +class cached_property(object): + def __init__(self, func): + self.func = func + def __get__(self, obj, cls): + val = self.func(obj) + setattr(obj, self.func.__name__, val) + return val + +class MyForm(object): + @cached_property + def changed_data(self): + return [1, 2, 3] + +def test_decorator_class(form): + f = MyForm() + # OK: cached_property is a descriptor; the actual runtime value is a list. + if "name" in f.changed_data: + pass + +# Class with dynamically added methods via setattr: we cannot statically +# determine its full interface, so we should not flag it. +class DynamicProxy(object): + def __init__(self, args): + self._args = args + +for method_name in ["__contains__", "__iter__", "__len__"]: + def wrapper(self, *args, __method_name=method_name): + pass + setattr(DynamicProxy, method_name, wrapper) + +def test_dynamic_methods(): + proxy = DynamicProxy(()) + # OK: __contains__ is added dynamically via setattr. + if "name" in proxy: + pass