Skip to content

Commit 35793a1

Browse files
RasmusWLRasmusWL
authored
Merge pull request github#5889 from japroc/python-clickhouse-driver
Python: Implement module ClickHouseDriver.qll
2 parents 8cbb3ca + 1e40213 commit 35793a1

File tree

7 files changed

+237
-0
lines changed

7 files changed

+237
-0
lines changed

‎python/ql/src/experimental/Security/CWE-089/ClickHouseSQLInjection.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
from django.conf.urls import url
2+
from clickhouse_driver import Client
3+
from clickhouse_driver import connect
4+
from aioch import Client as aiochClient
5+
6+
class MyClient(Client):
7+
def dummy(self):
8+
return None
9+
10+
def show_user(request, username):
11+
12+
# BAD -- Untrusted user input is directly injected into the sql query using async library 'aioch'
13+
aclient = aiochClient("localhost")
14+
progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
15+
16+
# BAD -- Untrusted user input is directly injected into the sql query using native client of library 'clickhouse_driver'
17+
Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
18+
19+
# GOOD -- query uses prepared statements
20+
query = "SELECT * FROM users WHERE username = %(username)s"
21+
Client('localhost').execute(query, {"username": username})
22+
23+
# BAD -- PEP249 interface
24+
conn = connect('clickhouse://localhost')
25+
cursor = conn.cursor()
26+
cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
27+
28+
urlpatterns = [url(r'^users/(?P<username>[^/]+)$', show_user)]

‎python/ql/src/experimental/Security/CWE-089/ClickHouseSQLInjection.qhelp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
If a database query (such as a SQL or NoSQL query) is built from
9+
user-provided data without sufficient sanitization, a user
10+
may be able to run malicious database queries.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Most database connector libraries offer a way of safely
17+
embedding untrusted data into a query by means of query parameters
18+
or prepared statements.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
In the following snippet, a user is fetched from a <code>ClickHouse</code> database
25+
using five different queries. In the "BAD" cases the query is built directly from user-controlled data.
26+
In the "GOOD" case the user-controlled data is safely embedded into the query by using query parameters.
27+
</p>
28+
29+
<p>
30+
In the first case, the query executed via aioch Client. aioch - is a module
31+
for asynchronous queries to database.
32+
</p>
33+
34+
<p>
35+
In the second and third cases, the connection is established via `Client` class.
36+
This class implement different method to execute a query.
37+
</p>
38+
39+
<p>
40+
In the forth case, good pattern is presented. Query parameters are send through
41+
second dict-like argument.
42+
</p>
43+
44+
<p>
45+
In the fifth case, there is example of PEP249 interface usage.
46+
</p>
47+
48+
<p>
49+
In the sixth case, there is custom Class usge which is a subclass of default Client.
50+
</p>
51+
52+
<sample src="ClickHouseSQLInjection.py" />
53+
</example>
54+
55+
<references>
56+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
57+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html">SQL Injection Prevention Cheat Sheet</a>.</li>
58+
</references>
59+
</qhelp>

‎python/ql/src/experimental/Security/CWE-089/ClickHouseSQLInjection.ql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* @id py/yandex/clickhouse-sql-injection
3+
* @name Clickhouse SQL query built from user-controlled sources
4+
* @description Building a SQL query from user-controlled sources is vulnerable to insertion of
5+
* malicious SQL code by the user.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @precision high
9+
* @tags security
10+
* external/cwe/cwe-089
11+
* external/owasp/owasp-a1
12+
*/
13+
14+
import python
15+
import experimental.semmle.python.frameworks.ClickHouseDriver
16+
import semmle.python.security.dataflow.SqlInjection
17+
import DataFlow::PathGraph
18+
19+
from SQLInjectionConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
20+
where config.hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "This SQL query depends on $@.", source.getNode(),
22+
"a user-provided value"

‎python/ql/src/experimental/semmle/python/frameworks/ClickHouseDriver.qll

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Provides classes modeling security-relevant aspects of `clickhouse-driver` and `aioch` PyPI packages.
3+
* See
4+
* - https://pypi.org/project/clickhouse-driver/
5+
* - https://pypi.org/project/aioch/
6+
* - https://clickhouse-driver.readthedocs.io/en/latest/
7+
*/
8+
9+
private import python
10+
private import semmle.python.Concepts
11+
private import semmle.python.ApiGraphs
12+
private import semmle.python.frameworks.PEP249
13+
14+
/**
15+
* Provides models for `clickhouse-driver` and `aioch` PyPI packages.
16+
* See
17+
* - https://pypi.org/project/clickhouse-driver/
18+
* - https://pypi.org/project/aioch/
19+
* - https://clickhouse-driver.readthedocs.io/en/latest/
20+
*/
21+
module ClickHouseDriver {
22+
/** Gets a reference to the `clickhouse_driver` module. */
23+
API::Node clickhouse_driver() { result = API::moduleImport("clickhouse_driver") }
24+
25+
/** Gets a reference to the `aioch` module. This module allows to make async db queries. */
26+
API::Node aioch() { result = API::moduleImport("aioch") }
27+
28+
/**
29+
* `clickhouse_driver` implements PEP249,
30+
* providing ways to execute SQL statements against a database.
31+
*/
32+
class ClickHouseDriverPEP249 extends PEP249ModuleApiNode {
33+
ClickHouseDriverPEP249() { this = clickhouse_driver() }
34+
}
35+
36+
module Client {
37+
/** Gets a reference to a Client call. */
38+
private DataFlow::Node client_ref() {
39+
result = clickhouse_driver().getMember("Client").getASubclass*().getAUse()
40+
or
41+
result = aioch().getMember("Client").getASubclass*().getAUse()
42+
}
43+
44+
/** A direct instantiation of `clickhouse_driver.Client`. */
45+
private class ClientInstantiation extends DataFlow::CallCfgNode {
46+
ClientInstantiation() { this.getFunction() = client_ref() }
47+
}
48+
49+
/** Gets a reference to an instance of `clickhouse_driver.Client`. */
50+
private DataFlow::LocalSourceNode instance(DataFlow::TypeTracker t) {
51+
t.start() and
52+
result instanceof ClientInstantiation
53+
or
54+
exists(DataFlow::TypeTracker t2 | result = instance(t2).track(t2, t))
55+
}
56+
57+
/** Gets a reference to an instance of `clickhouse_driver.Client`. */
58+
DataFlow::Node instance() { instance(DataFlow::TypeTracker::end()).flowsTo(result) }
59+
}
60+
61+
/** clickhouse_driver.Client execute methods */
62+
private string execute_function() {
63+
result in ["execute_with_progress", "execute", "execute_iter"]
64+
}
65+
66+
/** Gets a reference to the `clickhouse_driver.Client.execute` method */
67+
private DataFlow::LocalSourceNode clickhouse_execute(DataFlow::TypeTracker t) {
68+
t.startInAttr(execute_function()) and
69+
result = Client::instance()
70+
or
71+
exists(DataFlow::TypeTracker t2 | result = clickhouse_execute(t2).track(t2, t))
72+
}
73+
74+
/** Gets a reference to the `clickhouse_driver.Client.execute` method */
75+
DataFlow::Node clickhouse_execute() {
76+
clickhouse_execute(DataFlow::TypeTracker::end()).flowsTo(result)
77+
}
78+
79+
/** A call to the `clickhouse_driver.Client.execute` method */
80+
private class ExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {
81+
ExecuteCall() { this.getFunction() = clickhouse_execute() }
82+
83+
override DataFlow::Node getSql() { result.asCfgNode() = node.getArg(0) }
84+
}
85+
}

‎python/ql/test/experimental/semmle/python/frameworks/clickhouse-driver/ClickHouseDriver.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| ClickHouseDriver.py:15:22:15:106 | ControlFlowNode for Attribute() | ClickHouseDriver.py:15:52:15:105 | ControlFlowNode for BinaryExpr |
2+
| ClickHouseDriver.py:18:5:18:87 | ControlFlowNode for Attribute() | ClickHouseDriver.py:18:33:18:86 | ControlFlowNode for BinaryExpr |
3+
| ClickHouseDriver.py:22:5:22:62 | ControlFlowNode for Attribute() | ClickHouseDriver.py:22:33:22:37 | ControlFlowNode for query |
4+
| ClickHouseDriver.py:27:5:27:74 | ControlFlowNode for Attribute() | ClickHouseDriver.py:27:20:27:73 | ControlFlowNode for BinaryExpr |
5+
| ClickHouseDriver.py:30:5:30:89 | ControlFlowNode for Attribute() | ClickHouseDriver.py:30:35:30:88 | ControlFlowNode for BinaryExpr |

‎python/ql/test/experimental/semmle/python/frameworks/clickhouse-driver/ClickHouseDriver.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
from django.conf.urls import url
2+
from clickhouse_driver import Client
3+
from clickhouse_driver import connect
4+
from aioch import Client as aiochClient
5+
6+
# Dummy Client subclass
7+
class MyClient(Client):
8+
def dummy(self):
9+
return None
10+
11+
def show_user(request, username):
12+
13+
# BAD -- Untrusted user input is directly injected into the sql query using async library 'aioch'
14+
aclient = aiochClient("localhost")
15+
progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
16+
17+
# BAD -- Untrusted user input is directly injected into the sql query using native client of library 'clickhouse_driver'
18+
Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
19+
20+
# GOOD -- query uses prepared statements
21+
query = "SELECT * FROM users WHERE username = %(username)s"
22+
Client('localhost').execute(query, {"username": username})
23+
24+
# BAD -- Untrusted user input is directly injected into the sql query using PEP249 interface
25+
conn = connect('clickhouse://localhost')
26+
cursor = conn.cursor()
27+
cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
28+
29+
# BAD -- Untrusted user input is directly injected into the sql query using MyClient, which is a subclass of Client
30+
MyClient('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
31+
32+
urlpatterns = [url(r'^users/(?P<username>[^/]+)$', show_user)]

‎python/ql/test/experimental/semmle/python/frameworks/clickhouse-driver/ClickHouseDriver.ql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import python
2+
import experimental.semmle.python.frameworks.ClickHouseDriver
3+
import semmle.python.Concepts
4+
5+
from SqlExecution s
6+
select s, s.getSql()

0 commit comments

Comments
 (0)