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