Skip to content

Commit 2efa0ad

Browse files
japrocjaproc
committed
[C++] Implement module ClickHouseDriver.qll
1 parent 5016c64 commit 2efa0ad

File tree

4 files changed

+192
-0
lines changed

4 files changed

+192
-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+
def show_user(request, username):
7+
8+
# BAD -- async library 'aioch'
9+
aclient = aiochClient("localhost")
10+
progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
11+
12+
# BAD -- client excute
13+
client = Client('localhost')
14+
client.execute("SELECT * FROM users WHERE username = '%s'" % username)
15+
16+
# BAD -- client excute oneliner
17+
Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
18+
19+
# GOOD -- send username through params
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: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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 the ClickHouse database
25+
using five different queries. In bad patterns query is build by formatting
26+
string with user-controlled data. In a qood pattern user-supplied parameter
27+
is send through a second argument (params) which is dict. Following cases
28+
show different types of communication with ClickHouse database.
29+
</p>
30+
31+
<p>
32+
In the first case, the query executed via aioch Client. aioch - is a module
33+
for asynchronous queries to database.
34+
</p>
35+
36+
<p>
37+
In the second and third cases, the connection is established via `Client` class.
38+
This class implement different method to execute a query.
39+
</p>
40+
41+
<p>
42+
In the forth case, good pattern is presented. Query parameters are send through
43+
second dict-like argument.
44+
</p>
45+
46+
<p>
47+
In the fifth case, there is example of PEP249 interface usage.
48+
</p>
49+
50+
<sample src="ClickHouseSQLInjection.py" />
51+
</example>
52+
53+
<references>
54+
<li>Wikipedia: <a href="https://en.wikipedia.org/wiki/SQL_injection">SQL injection</a>.</li>
55+
<li>OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html">SQL Injection Prevention Cheat Sheet</a>.</li>
56+
</references>
57+
</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").getAUse()
40+
or
41+
result = aioch().getMember("Client").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+
}

0 commit comments

Comments
 (0)