From cb8f1b4593175bd4eb0f4bf962fbdaf61942e3d0 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 9 Nov 2021 11:49:31 +0100 Subject: [PATCH 1/8] Python: Add tests for aiopg --- .../frameworks/aiopg/ConceptsTest.expected | 0 .../frameworks/aiopg/ConceptsTest.ql | 2 ++ .../library-tests/frameworks/aiopg/test.py | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.expected create mode 100644 python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.ql create mode 100644 python/ql/test/library-tests/frameworks/aiopg/test.py diff --git a/python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.expected b/python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.ql b/python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.ql new file mode 100644 index 00000000000..b557a0bccb6 --- /dev/null +++ b/python/ql/test/library-tests/frameworks/aiopg/ConceptsTest.ql @@ -0,0 +1,2 @@ +import python +import experimental.meta.ConceptsTest diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py new file mode 100644 index 00000000000..e95eb12bc8f --- /dev/null +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -0,0 +1,19 @@ +import aiopg + +# Only a cursor can execute sql. +async def test_cursor(): + # Create connection directly + conn = await aiopg.connect() + cur = await conn.cursor() + await cur.execute("sql") # $ MISSING: getSql="sql" + + # Create connection via pool + async with aiopg.create_pool() as pool: + # Create Cursor via Connection + async with pool.acquire() as conn: + cur = await conn.cursor() + await cur.execute("sql") # $ MISSING: getSql="sql" + + # Create Cursor directly + async with pool.cursor() as cur: + await cur.execute("sql") # $ MISSING: getSql="sql" From cd332a75fc1916986855c3cd8870464d1228dd7e Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 9 Nov 2021 12:32:21 +0100 Subject: [PATCH 2/8] Python: model aiopg --- docs/codeql/support/reusables/frameworks.rst | 1 + python/ql/lib/semmle/python/Frameworks.qll | 1 + .../ql/lib/semmle/python/frameworks/Aiopg.qll | 74 +++++++++++++++++++ .../library-tests/frameworks/aiopg/test.py | 6 +- 4 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 python/ql/lib/semmle/python/frameworks/Aiopg.qll diff --git a/docs/codeql/support/reusables/frameworks.rst b/docs/codeql/support/reusables/frameworks.rst index b186c08c5d6..52e96f9af0c 100644 --- a/docs/codeql/support/reusables/frameworks.rst +++ b/docs/codeql/support/reusables/frameworks.rst @@ -183,6 +183,7 @@ Python built-in support pydantic, Utility library yarl, Utility library aioch, Database + aiopg, Database asyncpg, Database clickhouse-driver, Database mysql-connector-python, Database diff --git a/python/ql/lib/semmle/python/Frameworks.qll b/python/ql/lib/semmle/python/Frameworks.qll index 932d2279a5a..b0587f9d430 100644 --- a/python/ql/lib/semmle/python/Frameworks.qll +++ b/python/ql/lib/semmle/python/Frameworks.qll @@ -6,6 +6,7 @@ // `docs/codeql/support/reusables/frameworks.rst` private import semmle.python.frameworks.Aioch private import semmle.python.frameworks.Aiohttp +private import semmle.python.frameworks.Aiopg private import semmle.python.frameworks.Asyncpg private import semmle.python.frameworks.ClickhouseDriver private import semmle.python.frameworks.Cryptodome diff --git a/python/ql/lib/semmle/python/frameworks/Aiopg.qll b/python/ql/lib/semmle/python/frameworks/Aiopg.qll new file mode 100644 index 00000000000..49eeb0d1d1f --- /dev/null +++ b/python/ql/lib/semmle/python/frameworks/Aiopg.qll @@ -0,0 +1,74 @@ +/** + * Provides classes modeling security-relevant aspects of the `aiopg` PyPI package. + * See + * - https://aiopg.readthedocs.io/en/stable/index.html + * - https://pypi.org/project/aiopg/ + */ + +private import python +private import semmle.python.dataflow.new.DataFlow +private import semmle.python.Concepts +private import semmle.python.ApiGraphs + +/** Provides models for the `aiopg` PyPI package. */ +private module Aiopg { + private import semmle.python.internal.Awaited + + /** A `ConectionPool` is created when the result of `aiopg.create_pool()` is awaited. */ + API::Node connectionPool() { + result = API::moduleImport("aiopg").getMember("create_pool").getReturn().getAwaited() + } + + /** + * A `Connection` is created when + * - the result of `aiopg.connect()` is awaited. + * - the result of calling `aquire` on a `ConnectionPool` is awaited. + */ + API::Node connection() { + result = API::moduleImport("aiopg").getMember("connect").getReturn().getAwaited() + or + result = connectionPool().getMember("acquire").getReturn().getAwaited() + } + + /** + * A `Cursor` is created when + * - the result of calling `cursor` on a `ConnectionPool` is awaited. + * - the result of calling `cursor` on a `Connection` is awaited. + */ + API::Node cursor() { + result = connectionPool().getMember("cursor").getReturn().getAwaited() + or + result = connection().getMember("cursor").getReturn().getAwaited() + } + + class CursorExecuteCall extends SqlConstruction::Range, DataFlow::CallCfgNode { + CursorExecuteCall() { this = cursor().getMember("execute").getACall() } + + override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("operation")] } + } + + /** + * This is only needed to connect the argument to the execute call with the subsequnt awaiting. + * It should be obsolete once we have `API::CallNode` available. + */ + private DataFlow::TypeTrackingNode cursorExecuteCall(DataFlow::TypeTracker t, DataFlow::Node sql) { + // cursor created from connection + t.start() and + sql = result.(CursorExecuteCall).getSql() + or + exists(DataFlow::TypeTracker t2 | result = cursorExecuteCall(t2, sql).track(t2, t)) + } + + DataFlow::Node cursorExecuteCall(DataFlow::Node sql) { + cursorExecuteCall(DataFlow::TypeTracker::end(), sql).flowsTo(result) + } + + /** Awaiting the result of calling `execute` executes the query. */ + class AwaitedCursorExecuteCall extends SqlExecution::Range { + DataFlow::Node sql; + + AwaitedCursorExecuteCall() { this = awaited(cursorExecuteCall(sql)) } + + override DataFlow::Node getSql() { result = sql } + } +} diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index e95eb12bc8f..23129e7f503 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -5,15 +5,15 @@ async def test_cursor(): # Create connection directly conn = await aiopg.connect() cur = await conn.cursor() - await cur.execute("sql") # $ MISSING: getSql="sql" + await cur.execute("sql") # $ getSql="sql" constructedSql="sql" # Create connection via pool async with aiopg.create_pool() as pool: # Create Cursor via Connection async with pool.acquire() as conn: cur = await conn.cursor() - await cur.execute("sql") # $ MISSING: getSql="sql" + await cur.execute("sql") # $ getSql="sql" constructedSql="sql" # Create Cursor directly async with pool.cursor() as cur: - await cur.execute("sql") # $ MISSING: getSql="sql" + await cur.execute("sql") # $ getSql="sql" constructedSql="sql" From f53314019ac0893c47daeffe1ec2f96692c6c6e4 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 9 Nov 2021 12:42:03 +0100 Subject: [PATCH 3/8] Python: test `aiopg.sa` --- python/ql/test/library-tests/frameworks/aiopg/test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index 23129e7f503..f27e12b3f3b 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -17,3 +17,10 @@ async def test_cursor(): # Create Cursor directly async with pool.cursor() as cur: await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + +from aiopg.sa import create_engine + +async def test_engine(): + engine = await create_engine() + conn = await engine.acquire() + await conn.execute("sql") # $ MISSING: getSql="sql" constructedSql="sql" From a58c47b07bf24a0fe7895822ea9acbee70a04a13 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 9 Nov 2021 12:49:57 +0100 Subject: [PATCH 4/8] Python: model `aiopg.sa` --- .../ql/lib/semmle/python/frameworks/Aiopg.qll | 46 +++++++++++++++++++ .../library-tests/frameworks/aiopg/test.py | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/python/ql/lib/semmle/python/frameworks/Aiopg.qll b/python/ql/lib/semmle/python/frameworks/Aiopg.qll index 49eeb0d1d1f..6ae5b5a05e8 100644 --- a/python/ql/lib/semmle/python/frameworks/Aiopg.qll +++ b/python/ql/lib/semmle/python/frameworks/Aiopg.qll @@ -41,6 +41,7 @@ private module Aiopg { result = connection().getMember("cursor").getReturn().getAwaited() } + /** Calling `execute` on a `Cursor` constructs a query. */ class CursorExecuteCall extends SqlConstruction::Range, DataFlow::CallCfgNode { CursorExecuteCall() { this = cursor().getMember("execute").getACall() } @@ -71,4 +72,49 @@ private module Aiopg { override DataFlow::Node getSql() { result = sql } } + + /** An `Engine` is created when the result of calling `aiopg.sa.create_engine` is awaited. */ + API::Node engine() { + result = + API::moduleImport("aiopg").getMember("sa").getMember("create_engine").getReturn().getAwaited() + } + + /** + * A `SAConnection` is created when the result of calling `aquire` on an `Engine` is awaited. + */ + API::Node saConnection() { result = engine().getMember("acquire").getReturn().getAwaited() } + + /** Calling `execute` on a `SAConnection` constructs a query. */ + class SAConnectionExecuteCall extends SqlConstruction::Range, DataFlow::CallCfgNode { + SAConnectionExecuteCall() { this = saConnection().getMember("execute").getACall() } + + override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("query")] } + } + + /** + * This is only needed to connect the argument to the execute call with the subsequnt awaiting. + * It should be obsolete once we have `API::CallNode` available. + */ + private DataFlow::TypeTrackingNode saConnectionExecuteCall( + DataFlow::TypeTracker t, DataFlow::Node sql + ) { + // saConnection created from engine + t.start() and + sql = result.(SAConnectionExecuteCall).getSql() + or + exists(DataFlow::TypeTracker t2 | result = saConnectionExecuteCall(t2, sql).track(t2, t)) + } + + DataFlow::Node saConnectionExecuteCall(DataFlow::Node sql) { + saConnectionExecuteCall(DataFlow::TypeTracker::end(), sql).flowsTo(result) + } + + /** Awaiting the result of calling `execute` executes the query. */ + class AwaitedSAConnectionExecuteCall extends SqlExecution::Range { + DataFlow::Node sql; + + AwaitedSAConnectionExecuteCall() { this = awaited(saConnectionExecuteCall(sql)) } + + override DataFlow::Node getSql() { result = sql } + } } diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index f27e12b3f3b..d04d63e257e 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -23,4 +23,4 @@ from aiopg.sa import create_engine async def test_engine(): engine = await create_engine() conn = await engine.acquire() - await conn.execute("sql") # $ MISSING: getSql="sql" constructedSql="sql" + await conn.execute("sql") # $ getSql="sql" constructedSql="sql" From aa1541a5c3809e12f37188e62b5300ff792cac42 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Tue, 9 Nov 2021 12:57:36 +0100 Subject: [PATCH 5/8] Python: add changenote --- python/change-notes/2021-11-09-model-aiopg.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 python/change-notes/2021-11-09-model-aiopg.md diff --git a/python/change-notes/2021-11-09-model-aiopg.md b/python/change-notes/2021-11-09-model-aiopg.md new file mode 100644 index 00000000000..7bf78a8de01 --- /dev/null +++ b/python/change-notes/2021-11-09-model-aiopg.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added modeling of `aiopg` for sinks executing SQL. From a856395d565547b19cb6cc9cb47e666e09ff7a8f Mon Sep 17 00:00:00 2001 From: yoff Date: Wed, 10 Nov 2021 10:51:40 +0100 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Rasmus Wriedt Larsen --- python/ql/test/library-tests/frameworks/aiopg/test.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index d04d63e257e..f9b8476ea11 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -11,13 +11,20 @@ async def test_cursor(): async with aiopg.create_pool() as pool: # Create Cursor via Connection async with pool.acquire() as conn: - cur = await conn.cursor() - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + async with conn.cursor() as cur: + await cur.execute("sql") # $ getSql="sql" constructedSql="sql" # Create Cursor directly async with pool.cursor() as cur: await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + # variants using as few `async with` as possible + pool = await aiopg.create_pool() + conn = pool.acquire() + cur = await conn.cursor() + await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + +# Test SQLAlchemy integration from aiopg.sa import create_engine async def test_engine(): From c6d285dd2a80ae713e1399ce1efaf9b514878755 Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Wed, 10 Nov 2021 11:06:45 +0100 Subject: [PATCH 7/8] Python: Fix test --- python/ql/test/library-tests/frameworks/aiopg/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index f9b8476ea11..63bf141f52d 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -20,7 +20,7 @@ async def test_cursor(): # variants using as few `async with` as possible pool = await aiopg.create_pool() - conn = pool.acquire() + conn = await pool.acquire() cur = await conn.cursor() await cur.execute("sql") # $ getSql="sql" constructedSql="sql" From 92a7114b727ea8d9d508f6fe5f6c61da613c55cf Mon Sep 17 00:00:00 2001 From: Rasmus Lerchedahl Petersen Date: Wed, 10 Nov 2021 11:06:58 +0100 Subject: [PATCH 8/8] Python: Add API references --- .../ql/lib/semmle/python/frameworks/Aiopg.qll | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/Aiopg.qll b/python/ql/lib/semmle/python/frameworks/Aiopg.qll index 6ae5b5a05e8..927ae90ed05 100644 --- a/python/ql/lib/semmle/python/frameworks/Aiopg.qll +++ b/python/ql/lib/semmle/python/frameworks/Aiopg.qll @@ -14,7 +14,10 @@ private import semmle.python.ApiGraphs private module Aiopg { private import semmle.python.internal.Awaited - /** A `ConectionPool` is created when the result of `aiopg.create_pool()` is awaited. */ + /** + * A `ConectionPool` is created when the result of `aiopg.create_pool()` is awaited. + * See https://aiopg.readthedocs.io/en/stable/core.html#pool + */ API::Node connectionPool() { result = API::moduleImport("aiopg").getMember("create_pool").getReturn().getAwaited() } @@ -23,6 +26,7 @@ private module Aiopg { * A `Connection` is created when * - the result of `aiopg.connect()` is awaited. * - the result of calling `aquire` on a `ConnectionPool` is awaited. + * See https://aiopg.readthedocs.io/en/stable/core.html#connection */ API::Node connection() { result = API::moduleImport("aiopg").getMember("connect").getReturn().getAwaited() @@ -34,6 +38,7 @@ private module Aiopg { * A `Cursor` is created when * - the result of calling `cursor` on a `ConnectionPool` is awaited. * - the result of calling `cursor` on a `Connection` is awaited. + * See https://aiopg.readthedocs.io/en/stable/core.html#cursor */ API::Node cursor() { result = connectionPool().getMember("cursor").getReturn().getAwaited() @@ -41,7 +46,10 @@ private module Aiopg { result = connection().getMember("cursor").getReturn().getAwaited() } - /** Calling `execute` on a `Cursor` constructs a query. */ + /** + * Calling `execute` on a `Cursor` constructs a query. + * See https://aiopg.readthedocs.io/en/stable/core.html#aiopg.Cursor.execute + */ class CursorExecuteCall extends SqlConstruction::Range, DataFlow::CallCfgNode { CursorExecuteCall() { this = cursor().getMember("execute").getACall() } @@ -64,7 +72,10 @@ private module Aiopg { cursorExecuteCall(DataFlow::TypeTracker::end(), sql).flowsTo(result) } - /** Awaiting the result of calling `execute` executes the query. */ + /** + * Awaiting the result of calling `execute` executes the query. + * See https://aiopg.readthedocs.io/en/stable/core.html#aiopg.Cursor.execute + */ class AwaitedCursorExecuteCall extends SqlExecution::Range { DataFlow::Node sql; @@ -73,7 +84,10 @@ private module Aiopg { override DataFlow::Node getSql() { result = sql } } - /** An `Engine` is created when the result of calling `aiopg.sa.create_engine` is awaited. */ + /** + * An `Engine` is created when the result of calling `aiopg.sa.create_engine` is awaited. + * See https://aiopg.readthedocs.io/en/stable/sa.html#engine + */ API::Node engine() { result = API::moduleImport("aiopg").getMember("sa").getMember("create_engine").getReturn().getAwaited() @@ -81,10 +95,14 @@ private module Aiopg { /** * A `SAConnection` is created when the result of calling `aquire` on an `Engine` is awaited. + * See https://aiopg.readthedocs.io/en/stable/sa.html#connection */ API::Node saConnection() { result = engine().getMember("acquire").getReturn().getAwaited() } - /** Calling `execute` on a `SAConnection` constructs a query. */ + /** + * Calling `execute` on a `SAConnection` constructs a query. + * See https://aiopg.readthedocs.io/en/stable/sa.html#aiopg.sa.SAConnection.execute + */ class SAConnectionExecuteCall extends SqlConstruction::Range, DataFlow::CallCfgNode { SAConnectionExecuteCall() { this = saConnection().getMember("execute").getACall() } @@ -109,7 +127,10 @@ private module Aiopg { saConnectionExecuteCall(DataFlow::TypeTracker::end(), sql).flowsTo(result) } - /** Awaiting the result of calling `execute` executes the query. */ + /** + * Awaiting the result of calling `execute` executes the query. + * See https://aiopg.readthedocs.io/en/stable/sa.html#aiopg.sa.SAConnection.execute + */ class AwaitedSAConnectionExecuteCall extends SqlExecution::Range { DataFlow::Node sql;