diff --git a/python/ql/lib/semmle/python/frameworks/PEP249.qll b/python/ql/lib/semmle/python/frameworks/PEP249.qll index 1eecc12b3d1e..1dcaa4173188 100644 --- a/python/ql/lib/semmle/python/frameworks/PEP249.qll +++ b/python/ql/lib/semmle/python/frameworks/PEP249.qll @@ -8,6 +8,7 @@ private import semmle.python.dataflow.new.DataFlow private import semmle.python.dataflow.new.RemoteFlowSources private import semmle.python.Concepts private import semmle.python.ApiGraphs +private import semmle.python.dataflow.new.internal.DataFlowDispatch as DataFlowDispatch /** * Provides classes modeling database interfaces following PEP 249. @@ -212,6 +213,74 @@ module PEP249 { ConnectCall() { this.getFunction() = connect() } } + /** + * Holds if class `cls` stores a PEP 249 database connection to `self.` + * in its `__init__` method, via a direct call to a `connect` function. + */ + private predicate classStoresConnectionInInit(Class cls, string attrName) { + exists(Function init, DataFlow::AttrWrite store | + cls.getAMethod() = init and + init.getName() = "__init__" and + store.getAttributeName() = attrName and + store.getObject().asCfgNode().getNode().(Name).getVariable() = + init.getArg(0).asName().getVariable() and + store.getValue() instanceof ConnectCall + ) + } + + /** + * A read of a connection-holding attribute within a method of a class whose + * `__init__` stores a PEP 249 connection in that attribute. + * + * This recognizes patterns such as: + * ```python + * class Wrapper: + * def __init__(self): + * self._conn = dbapi.connect(...) + * def get_connection(self): + * return self._conn # <-- recognized as a connection source + * ``` + * Because the `AttrRead` node for `self._conn` inside `get_connection` is + * also the `ExtractedReturnNode` for that statement, the existing TypeTracker + * `returnStep` automatically propagates the connection type to all call sites + * of `get_connection`. + */ + private class ConnectionGetterAttributeRead extends InstanceSource, DataFlow::AttrRead { + ConnectionGetterAttributeRead() { + exists(Class cls, Function method, string attrName | + classStoresConnectionInInit(cls, attrName) and + cls.getAMethod() = method and + method.getName() != "__init__" and + this.getAttributeName() = attrName and + this.getObject().asCfgNode().getNode().(Name).getVariable() = + method.getArg(0).asName().getVariable() + ) + } + } + + /** + * An attribute access on a constructor-call result that directly reads the + * connection-holding attribute. + * + * This recognizes patterns such as: + * ```python + * class Wrapper: + * def __init__(self): + * self._conn = dbapi.connect(...) + * + * conn = Wrapper()._conn # <-- recognized as a connection source + * ``` + */ + private class ConnectionConstructorAttributeRead extends InstanceSource, DataFlow::AttrRead { + ConnectionConstructorAttributeRead() { + exists(Class cls, string attrName | + classStoresConnectionInInit(cls, attrName) and + this.getAttributeName() = attrName and + DataFlowDispatch::resolveClassCall(this.getObject().asCfgNode().(CallNode), cls) + ) + } + } + /** Gets a reference to a database connection (following PEP 249). */ private DataFlow::TypeTrackingNode instance(DataFlow::TypeTracker t) { t.start() and diff --git a/python/ql/src/change-notes/2026-05-21-pep249-class-wrapper-connections.md b/python/ql/src/change-notes/2026-05-21-pep249-class-wrapper-connections.md new file mode 100644 index 000000000000..9d87a3ab79c5 --- /dev/null +++ b/python/ql/src/change-notes/2026-05-21-pep249-class-wrapper-connections.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Improved detection of SQL injection and other PEP 249 database-related vulnerabilities when a database connection is stored in a class instance attribute and accessed through a getter method or direct attribute read. For example, patterns like `self._conn = dbapi.connect(...)` in `__init__` followed by `return self._conn` in a getter method, or `MyClass()._conn`, are now correctly recognized as PEP 249 connection sources. diff --git a/python/ql/test/library-tests/frameworks/hdbcli/pep249.py b/python/ql/test/library-tests/frameworks/hdbcli/pep249.py index 713f15cb6d4f..5016b567a7f3 100644 --- a/python/ql/test/library-tests/frameworks/hdbcli/pep249.py +++ b/python/ql/test/library-tests/frameworks/hdbcli/pep249.py @@ -7,3 +7,49 @@ cursor.executemany("some sql", (42,)) # $ getSql="some sql" cursor.close() + + +# --------------------------------------------------------------------------- +# Connection stored in a class attribute and accessed via various patterns +# --------------------------------------------------------------------------- + + +class WrapperA: + def __init__(self): + self._conn = dbapi.connect(address="hostname", port=300, user="username", pass_arg="testpass") + + def get_connection(self): + return self._conn + + +# Getter called on a fresh constructor result +conn_a1 = WrapperA().get_connection() +cursor_a1 = conn_a1.cursor() +cursor_a1.execute("some sql", (42,)) # $ getSql="some sql" + +# Getter called via a stored wrapper instance +wrapper_instance = WrapperA() +conn_a2 = wrapper_instance.get_connection() +cursor_a2 = conn_a2.cursor() +cursor_a2.execute("some sql", (42,)) # $ getSql="some sql" + +# Direct attribute access on a fresh constructor result +conn_b = WrapperA()._conn +cursor_b = conn_b.cursor() +cursor_b.execute("some sql", (42,)) # $ getSql="some sql" + + +class WrapperB: + """Stores the connection under a different attribute name.""" + + def __init__(self): + self._hana = dbapi.connect(address="hostname", port=300, user="username", pass_arg="testpass") + + def cursor(self): + return self._hana.cursor() + + +# Direct attribute access on a stored instance (mirrors hdb_con3 in the issue) +conn_c = WrapperB()._hana +cursor_c = conn_c.cursor() +cursor_c.execute("some sql", (42,)) # $ getSql="some sql"