diff --git a/python/ql/lib/change-notes/2022-05-02-deprecate-sqlconstruction.md b/python/ql/lib/change-notes/2022-05-02-deprecate-sqlconstruction.md new file mode 100644 index 00000000000..4219fd12d9c --- /dev/null +++ b/python/ql/lib/change-notes/2022-05-02-deprecate-sqlconstruction.md @@ -0,0 +1,4 @@ +--- +category: deprecated +--- +The `SqlConstruction` class and module from `Concepts.qll` has been deprecated. Use `SqlExecution` from the same file instead. \ No newline at end of file diff --git a/python/ql/lib/semmle/python/Concepts.qll b/python/ql/lib/semmle/python/Concepts.qll index e734105cd2e..6154d24628b 100644 --- a/python/ql/lib/semmle/python/Concepts.qll +++ b/python/ql/lib/semmle/python/Concepts.qll @@ -308,36 +308,19 @@ module CodeExecution { } } -/** - * A data-flow node that constructs an SQL statement. - * - * Often, it is worthy of an alert if an SQL statement is constructed such that - * executing it would be a security risk. - * - * If it is important that the SQL statement is indeed executed, then use `SQLExecution`. - * - * Extend this class to refine existing API models. If you want to model new APIs, - * extend `SqlConstruction::Range` instead. - */ -class SqlConstruction extends DataFlow::Node instanceof SqlConstruction::Range { +/** DEPRECATED: Use `SqlExecution` instead. */ +deprecated class SqlConstruction extends DataFlow::Node instanceof SqlConstruction::Range { /** Gets the argument that specifies the SQL statements to be constructed. */ DataFlow::Node getSql() { result = super.getSql() } } -/** Provides a class for modeling new SQL execution APIs. */ -module SqlConstruction { - /** - * A data-flow node that constructs an SQL statement. - * - * Often, it is worthy of an alert if an SQL statement is constructed such that - * executing it would be a security risk. - * - * If it is important that the SQL statement is indeed executed, then use `SQLExecution`. - * - * Extend this class to model new APIs. If you want to refine existing API models, - * extend `SqlConstruction` instead. - */ - abstract class Range extends DataFlow::Node { +/** + * DEPRECATED: Use `SqlExecution` instead. + * Provides a class for modeling new SQL execution APIs. + */ +deprecated module SqlConstruction { + /** DEPRECATED: Use `SqlExecution::Range` instead. */ + abstract deprecated class Range extends DataFlow::Node { /** Gets the argument that specifies the SQL statements to be constructed. */ abstract DataFlow::Node getSql(); } @@ -346,9 +329,6 @@ module SqlConstruction { /** * A data-flow node that executes SQL statements. * - * If the context of interest is such that merely constructing an SQL statement - * would be valuabe to report, then consider using `SqlConstruction`. - * * Extend this class to refine existing API models. If you want to model new APIs, * extend `SqlExecution::Range` instead. */ @@ -362,9 +342,6 @@ module SqlExecution { /** * A data-flow node that executes SQL statements. * - * If the context of interest is such that merely constructing an SQL statement - * would be valuabe to report, then consider using `SqlConstruction`. - * * Extend this class to model new APIs. If you want to refine existing API models, * extend `SqlExecution` instead. */ diff --git a/python/ql/lib/semmle/python/frameworks/Aiomysql.qll b/python/ql/lib/semmle/python/frameworks/Aiomysql.qll index 112dc58d061..afff79783c2 100644 --- a/python/ql/lib/semmle/python/frameworks/Aiomysql.qll +++ b/python/ql/lib/semmle/python/frameworks/Aiomysql.qll @@ -50,7 +50,7 @@ private module Aiomysql { * A query. Calling `execute` on a `Cursor` constructs a query. * See https://aiomysql.readthedocs.io/en/stable/cursors.html#Cursor.execute */ - class CursorExecuteCall extends SqlConstruction::Range, API::CallNode { + class CursorExecuteCall extends SqlExecution::Range, API::CallNode { CursorExecuteCall() { this = cursor().getMember("execute").getACall() } override DataFlow::Node getSql() { result = this.getParameter(0, "operation").getARhs() } @@ -91,7 +91,7 @@ private module Aiomysql { * A query. Calling `execute` on a `SAConnection` constructs a query. * See https://aiomysql.readthedocs.io/en/stable/sa.html#aiomysql.sa.SAConnection.execute */ - class SAConnectionExecuteCall extends SqlConstruction::Range, API::CallNode { + class SAConnectionExecuteCall extends SqlExecution::Range, API::CallNode { SAConnectionExecuteCall() { this = saConnection().getMember("execute").getACall() } override DataFlow::Node getSql() { result = this.getParameter(0, "query").getARhs() } diff --git a/python/ql/lib/semmle/python/frameworks/Aiopg.qll b/python/ql/lib/semmle/python/frameworks/Aiopg.qll index 1a60c433150..67ba3c80db0 100644 --- a/python/ql/lib/semmle/python/frameworks/Aiopg.qll +++ b/python/ql/lib/semmle/python/frameworks/Aiopg.qll @@ -50,7 +50,7 @@ private module Aiopg { * 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, API::CallNode { + class CursorExecuteCall extends SqlExecution::Range, API::CallNode { CursorExecuteCall() { this = cursor().getMember("execute").getACall() } override DataFlow::Node getSql() { result = this.getParameter(0, "operation").getARhs() } @@ -87,7 +87,7 @@ private module Aiopg { * 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, API::CallNode { + class SAConnectionExecuteCall extends SqlExecution::Range, API::CallNode { SAConnectionExecuteCall() { this = saConnection().getMember("execute").getACall() } override DataFlow::Node getSql() { result = this.getParameter(0, "query").getARhs() } diff --git a/python/ql/lib/semmle/python/frameworks/Asyncpg.qll b/python/ql/lib/semmle/python/frameworks/Asyncpg.qll index 2b2b9ed2e1b..77678c92b51 100644 --- a/python/ql/lib/semmle/python/frameworks/Asyncpg.qll +++ b/python/ql/lib/semmle/python/frameworks/Asyncpg.qll @@ -56,7 +56,7 @@ private module Asyncpg { * The creation of the `Cursor` executes the query. */ module Cursor { - class CursorConstruction extends SqlConstruction::Range, API::CallNode { + class CursorConstruction extends SqlExecution::Range, API::CallNode { CursorConstruction() { this = ModelOutput::getATypeNode("asyncpg", "Connection").getMember("cursor").getACall() } diff --git a/python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll b/python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll index c1712e7e7eb..8a7e2f176e8 100644 --- a/python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll +++ b/python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll @@ -323,7 +323,7 @@ module SqlAlchemy { * A construction of a `sqlalchemy.sql.expression.TextClause`, which represents a * textual SQL string directly. */ - abstract class TextClauseConstruction extends SqlConstruction::Range, DataFlow::CallCfgNode { + abstract class TextClauseConstruction extends SqlExecution::Range, DataFlow::CallCfgNode { /** Gets the argument that specifies the SQL text. */ override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("text")] } } diff --git a/python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll b/python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll index 756a1f6b773..9e3e2749a2b 100644 --- a/python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll +++ b/python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll @@ -43,9 +43,10 @@ module SqlInjection { class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { } /** + * DEPRECATED: Use `SqlExecutionAsSink` instead. * A SQL statement of a SQL construction, considered as a flow sink. */ - class SqlConstructionAsSink extends Sink { + deprecated class SqlConstructionAsSink extends Sink { SqlConstructionAsSink() { this = any(SqlConstruction c).getSql() } } diff --git a/python/ql/test/experimental/meta/ConceptsTest.qll b/python/ql/test/experimental/meta/ConceptsTest.qll index 8f9435f633f..bdf71059680 100644 --- a/python/ql/test/experimental/meta/ConceptsTest.qll +++ b/python/ql/test/experimental/meta/ConceptsTest.qll @@ -128,24 +128,6 @@ class CodeExecutionTest extends InlineExpectationsTest { } } -class SqlConstructionTest extends InlineExpectationsTest { - SqlConstructionTest() { this = "SqlConstructionTest" } - - override string getARelevantTag() { result = "constructedSql" } - - override predicate hasActualResult(Location location, string element, string tag, string value) { - exists(location.getFile().getRelativePath()) and - exists(SqlConstruction e, DataFlow::Node sql | - exists(location.getFile().getRelativePath()) and - sql = e.getSql() and - location = e.getLocation() and - element = sql.toString() and - value = prettyNodeForInlineTest(sql) and - tag = "constructedSql" - ) - } -} - class SqlExecutionTest extends InlineExpectationsTest { SqlExecutionTest() { this = "SqlExecutionTest" } diff --git a/python/ql/test/library-tests/frameworks/aiomysql/test.py b/python/ql/test/library-tests/frameworks/aiomysql/test.py index 782fa6cb7bf..5a06b46b9f1 100644 --- a/python/ql/test/library-tests/frameworks/aiomysql/test.py +++ b/python/ql/test/library-tests/frameworks/aiomysql/test.py @@ -5,24 +5,24 @@ async def test_cursor(): # Create connection directly conn = await aiomysql.connect() cur = await conn.cursor() - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Create connection via pool async with aiomysql.create_pool() as pool: # Create Cursor via Connection async with pool.acquire() as conn: async with conn.cursor() as cur: - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Create Cursor directly async with pool.cursor() as cur: - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # variants using as few `async with` as possible pool = await aiomysql.create_pool() conn = await pool.acquire() cur = await conn.cursor() - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Test SQLAlchemy integration from aiomysql.sa import create_engine @@ -30,4 +30,4 @@ from aiomysql.sa import create_engine async def test_engine(): engine = await create_engine() conn = await engine.acquire() - await conn.execute("sql") # $ getSql="sql" constructedSql="sql" + await conn.execute("sql") # $ getSql="sql" diff --git a/python/ql/test/library-tests/frameworks/aiopg/test.py b/python/ql/test/library-tests/frameworks/aiopg/test.py index 63bf141f52d..1d085066688 100644 --- a/python/ql/test/library-tests/frameworks/aiopg/test.py +++ b/python/ql/test/library-tests/frameworks/aiopg/test.py @@ -5,24 +5,24 @@ async def test_cursor(): # Create connection directly conn = await aiopg.connect() cur = await conn.cursor() - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Create connection via pool async with aiopg.create_pool() as pool: # Create Cursor via Connection async with pool.acquire() as conn: async with conn.cursor() as cur: - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Create Cursor directly async with pool.cursor() as cur: - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # variants using as few `async with` as possible pool = await aiopg.create_pool() conn = await pool.acquire() cur = await conn.cursor() - await cur.execute("sql") # $ getSql="sql" constructedSql="sql" + await cur.execute("sql") # $ getSql="sql" # Test SQLAlchemy integration from aiopg.sa import create_engine @@ -30,4 +30,4 @@ from aiopg.sa import create_engine async def test_engine(): engine = await create_engine() conn = await engine.acquire() - await conn.execute("sql") # $ getSql="sql" constructedSql="sql" + await conn.execute("sql") # $ getSql="sql" diff --git a/python/ql/test/library-tests/frameworks/asyncpg/test.py b/python/ql/test/library-tests/frameworks/asyncpg/test.py index 572fc454bed..7e2687e5766 100644 --- a/python/ql/test/library-tests/frameworks/asyncpg/test.py +++ b/python/ql/test/library-tests/frameworks/asyncpg/test.py @@ -43,20 +43,20 @@ async def test_cursor(): try: async with conn.transaction(): - cursor = await conn.cursor("sql") # $ getSql="sql" constructedSql="sql" + cursor = await conn.cursor("sql") # $ getSql="sql" await cursor.fetch() pstmt = await conn.prepare("psql") # $ getSql="psql" pcursor = await pstmt.cursor() # $ getSql="psql" await pcursor.fetch() - async for record in conn.cursor("sql"): # $ getSql="sql" constructedSql="sql" + async for record in conn.cursor("sql"): # $ getSql="sql" pass async for record in pstmt.cursor(): # $ getSql="psql" pass - cursor_factory = conn.cursor("sql") # $ constructedSql="sql" + cursor_factory = conn.cursor("sql") # $ getSql="sql" cursor = await cursor_factory # $ getSql="sql" pcursor_factory = pstmt.cursor() diff --git a/python/ql/test/library-tests/frameworks/flask_sqlalchemy/SqlExecution.py b/python/ql/test/library-tests/frameworks/flask_sqlalchemy/SqlExecution.py index 39cd49195d2..19a1349ffb6 100644 --- a/python/ql/test/library-tests/frameworks/flask_sqlalchemy/SqlExecution.py +++ b/python/ql/test/library-tests/frameworks/flask_sqlalchemy/SqlExecution.py @@ -12,7 +12,7 @@ db = SQLAlchemy(app) # - https://github.com/pallets/flask-sqlalchemy/blob/931ec00d1e27f51508e05706eef41cc4419a0b32/src/flask_sqlalchemy/__init__.py#L765 # - https://github.com/pallets/flask-sqlalchemy/blob/931ec00d1e27f51508e05706eef41cc4419a0b32/src/flask_sqlalchemy/__init__.py#L99-L109 -assert str(type(db.text("Foo"))) == "" # $ constructedSql="Foo" +assert str(type(db.text("Foo"))) == "" # $ getSql="Foo" # also has engine/session instantiated @@ -44,8 +44,8 @@ assert result.fetchall() == [("Foo",)] # text -t = db.text("foo") # $ constructedSql="foo" +t = db.text("foo") # $ getSql="foo" assert isinstance(t, sqlalchemy.sql.expression.TextClause) -t = db.text(text="foo") # $ constructedSql="foo" +t = db.text(text="foo") # $ getSql="foo" assert isinstance(t, sqlalchemy.sql.expression.TextClause) diff --git a/python/ql/test/library-tests/frameworks/sqlalchemy/SqlExecution.py b/python/ql/test/library-tests/frameworks/sqlalchemy/SqlExecution.py index fe75fc17d5c..7d9c478e27b 100644 --- a/python/ql/test/library-tests/frameworks/sqlalchemy/SqlExecution.py +++ b/python/ql/test/library-tests/frameworks/sqlalchemy/SqlExecution.py @@ -46,7 +46,7 @@ with engine.begin() as connection: connection.execute("some sql") # $ getSql="some sql" # Injection requiring the text() taint-step -t = text("some sql") # $ constructedSql="some sql" +t = text("some sql") # $ getSql="some sql" session.query(User).filter(t) session.query(User).group_by(User.id).having(t) session.query(User).group_by(t).first() diff --git a/python/ql/test/library-tests/frameworks/sqlalchemy/new_tests.py b/python/ql/test/library-tests/frameworks/sqlalchemy/new_tests.py index 6b11ee5d070..9cb13d15e05 100644 --- a/python/ql/test/library-tests/frameworks/sqlalchemy/new_tests.py +++ b/python/ql/test/library-tests/frameworks/sqlalchemy/new_tests.py @@ -6,7 +6,7 @@ import sqlalchemy.orm # either v1.4 or v2.0, such that we cover both. raw_sql = "select 'FOO'" -text_sql = sqlalchemy.text(raw_sql) # $ constructedSql=raw_sql +text_sql = sqlalchemy.text(raw_sql) # $ getSql=raw_sql Base = sqlalchemy.orm.declarative_base() @@ -176,7 +176,7 @@ assert session.query(For14).all()[0].id == 14 # and now we can do the actual querying -text_foo = sqlalchemy.text("'FOO'") # $ constructedSql="'FOO'" +text_foo = sqlalchemy.text("'FOO'") # $ getSql="'FOO'" # filter_by is only vulnerable to injection if sqlalchemy.text is used, which is evident # from the logs produced if this file is run @@ -305,7 +305,7 @@ with engine.connect() as conn: assert scalar_result == "FOO" # This is a contrived example - select = sqlalchemy.select(sqlalchemy.text("'BAR'")) # $ constructedSql="'BAR'" + select = sqlalchemy.select(sqlalchemy.text("'BAR'")) # $ getSql="'BAR'" result = conn.execute(select) # $ getSql=select assert result.fetchall() == [("BAR",)] diff --git a/python/ql/test/library-tests/frameworks/sqlalchemy/taint_test.py b/python/ql/test/library-tests/frameworks/sqlalchemy/taint_test.py index 884d30d3672..9d346d41979 100644 --- a/python/ql/test/library-tests/frameworks/sqlalchemy/taint_test.py +++ b/python/ql/test/library-tests/frameworks/sqlalchemy/taint_test.py @@ -8,14 +8,14 @@ def test_taint(): ensure_tainted(ts) # $ tainted - t1 = sqlalchemy.text(ts) # $ constructedSql=ts - t2 = sqlalchemy.text(text=ts) # $ constructedSql=ts - t3 = sqlalchemy.sql.text(ts) # $ constructedSql=ts - t4 = sqlalchemy.sql.text(text=ts) # $ constructedSql=ts - t5 = sqlalchemy.sql.expression.text(ts) # $ constructedSql=ts - t6 = sqlalchemy.sql.expression.text(text=ts) # $ constructedSql=ts - t7 = sqlalchemy.sql.expression.TextClause(ts) # $ constructedSql=ts - t8 = sqlalchemy.sql.expression.TextClause(text=ts) # $ constructedSql=ts + t1 = sqlalchemy.text(ts) # $ getSql=ts + t2 = sqlalchemy.text(text=ts) # $ getSql=ts + t3 = sqlalchemy.sql.text(ts) # $ getSql=ts + t4 = sqlalchemy.sql.text(text=ts) # $ getSql=ts + t5 = sqlalchemy.sql.expression.text(ts) # $ getSql=ts + t6 = sqlalchemy.sql.expression.text(text=ts) # $ getSql=ts + t7 = sqlalchemy.sql.expression.TextClause(ts) # $ getSql=ts + t8 = sqlalchemy.sql.expression.TextClause(text=ts) # $ getSql=ts # Since we flag user-input to a TextClause with its' own query, we don't want to # have a taint-step for it as that would lead to us also giving an alert for normal