hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-21 11:16:30 +01:00
Code Issues Packages Projects Releases Wiki Activity

deprecate SqlConstruction

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2022-05-01 17:57:48 +02:00
parent 6c67e51ec3
commit c0eca0d09a
15 changed files with 49 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
python/ql/lib/change-notes/2022-05-02-deprecate-sqlconstruction.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: deprecated
---
The `SqlConstruction` class and module from `Concepts.qll` has been deprecated. Use `SqlExecution` from the same file instead.

41
python/ql/lib/semmle/python/Concepts.qll
View File

@@ -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.
*/

4
python/ql/lib/semmle/python/frameworks/Aiomysql.qll
View File

@@ -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() }

4
python/ql/lib/semmle/python/frameworks/Aiopg.qll
View File

@@ -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() }

2
python/ql/lib/semmle/python/frameworks/Asyncpg.qll
View File

@@ -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()
}

2
python/ql/lib/semmle/python/frameworks/SqlAlchemy.qll
View File

@@ -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")] }
}

3
python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll
View File

@@ -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() }
}

18
python/ql/test/experimental/meta/ConceptsTest.qll
View File

@@ -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" }

10
python/ql/test/library-tests/frameworks/aiomysql/test.py
View File

@@ -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"

10
python/ql/test/library-tests/frameworks/aiopg/test.py
View File

@@ -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"

6
python/ql/test/library-tests/frameworks/asyncpg/test.py
View File

@@ -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()

6
python/ql/test/library-tests/frameworks/flask_sqlalchemy/SqlExecution.py
View File

@@ -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"))) == "<class 'sqlalchemy.sql.elements.TextClause'>" # $ constructedSql="Foo"
assert str(type(db.text("Foo"))) == "<class 'sqlalchemy.sql.elements.TextClause'>" # $ 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)

2
python/ql/test/library-tests/frameworks/sqlalchemy/SqlExecution.py
View File

@@ -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()

6
python/ql/test/library-tests/frameworks/sqlalchemy/new_tests.py
View File

@@ -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",)]

16
python/ql/test/library-tests/frameworks/sqlalchemy/taint_test.py
View File

@@ -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