Skip to content

Commit 65d3373

Browse files
authored
Merge pull request #6727 from RasmusWL/fix-sqlalchemy-query
Python: Merge SQLAlchemy TextClause injection into `py/sql-injection`
2 parents d62f76a + d44f279 commit 65d3373

13 files changed

+55
-245
lines changed

python/change-notes/2021-09-02-add-SQLAlchemyTextClauseInjection-query.md

Lines changed: 0 additions & 2 deletions
This file was deleted.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Expanded the query _SQL query built from user-controlled sources_ (`py/sql-injection`) to alert if user-input is added to a TextClause from SQLAlchemy, since that can lead to SQL injection.

python/ql/lib/semmle/python/security/dataflow/SQLAlchemyTextClause.qll

Lines changed: 0 additions & 35 deletions
This file was deleted.

python/ql/lib/semmle/python/security/dataflow/SQLAlchemyTextClauseCustomizations.qll

Lines changed: 0 additions & 56 deletions
This file was deleted.

python/ql/lib/semmle/python/security/dataflow/SqlInjectionCustomizations.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import semmle.python.dataflow.new.DataFlow
99
private import semmle.python.Concepts
1010
private import semmle.python.dataflow.new.RemoteFlowSources
1111
private import semmle.python.dataflow.new.BarrierGuards
12+
private import semmle.python.frameworks.SqlAlchemy
1213

1314
/**
1415
* Provides default sources, sinks and sanitizers for detecting
@@ -48,6 +49,13 @@ module SqlInjection {
4849
SqlExecutionAsSink() { this = any(SqlExecution e).getSql() }
4950
}
5051

52+
/**
53+
* The text argument of a SQLAlchemy TextClause construction, considered as a flow sink.
54+
*/
55+
class TextArgAsSink extends Sink {
56+
TextArgAsSink() { this = any(SqlAlchemy::TextClause::TextClauseConstruction tcc).getTextArg() }
57+
}
58+
5159
/**
5260
* A comparison with a constant string, considered as a sanitizer-guard.
5361
*/

python/ql/src/Security/CWE-089/SQLAlchemyTextClauseInjection.qhelp

Lines changed: 0 additions & 53 deletions
This file was deleted.

python/ql/src/Security/CWE-089/SQLAlchemyTextClauseInjection.ql

Lines changed: 0 additions & 23 deletions
This file was deleted.

python/ql/src/Security/CWE-089/SqlInjection.qhelp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ If a database query (such as a SQL or NoSQL query) is built from
99
user-provided data without sufficient sanitization, a user
1010
may be able to run malicious database queries.
1111
</p>
12+
13+
<p>
14+
This also includes using the <code>TextClause</code> class in the
15+
<code><a href="https://pypi.org/project/SQLAlchemy/">SQLAlchemy</a></code> PyPI package,
16+
which is used to represent a literal SQL fragment and is inserted directly into the
17+
final SQL when used in a query built using the ORM.
18+
</p>
1219
</overview>
1320

1421
<recommendation>
@@ -52,5 +59,6 @@ vulnerable to SQL injection attacks. In this example, if <code>username</code> w
5259
<references>
5360
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
5461
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html">SQL Injection Prevention Cheat Sheet</a>.</li>
62+
<li><a href="https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.text.params.text">SQLAlchemy documentation for TextClause</a>.</li>
5563
</references>
5664
</qhelp>

python/ql/src/Security/CWE-089/examples/sqlalchemy_textclause_injection.py

Lines changed: 0 additions & 34 deletions
This file was deleted.

python/ql/test/query-tests/Security/CWE-089-SQLAlchemyTextClauseInjection/SQLAlchemyTextClauseInjection.expected

Lines changed: 0 additions & 41 deletions
This file was deleted.

0 commit comments

Comments
 (0)