mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
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.
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user