ClickHouseSQLInjection.qll : add tests

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

@@ -9,18 +9,14 @@ class MyClient(Client):
 
 def show_user(request, username):
 
-    # BAD -- async library 'aioch'
+    # BAD -- Untrusted user input is directly injected into the sql query using async library 'aioch'
     aclient = aiochClient("localhost")
     progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
 
-    # BAD -- client excute
-    client = Client('localhost')
-    client.execute("SELECT * FROM users WHERE username = '%s'" % username)
-
-    # BAD -- client excute oneliner
+    # BAD -- Untrusted user input is directly injected into the sql query using native client of library 'clickhouse_driver'
     Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
 
-    # GOOD -- send username through params
+    # GOOD -- query uses prepared statements
     query = "SELECT * FROM users WHERE username = %(username)s"
     Client('localhost').execute(query, {"username": username})
 
@@ -29,7 +25,4 @@ def show_user(request, username):
     cursor = conn.cursor()
     cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
 
-    # BAD -- MyClient is a subclass of Client
-    MyClient('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
-
 urlpatterns = [url(r'^users/(?P<username>[^/]+)$', show_user)]

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

@@ -21,11 +21,9 @@ or prepared statements.
 
 <example>
 <p>
-In the following snippet, a user is fetched from the ClickHouse database
-using five different queries. In bad patterns query is build by formatting
-string with user-controlled data. In a qood pattern user-supplied parameter
-is send through a second argument (params) which is dict. Following cases
-show different types of communication with ClickHouse database.
+In the following snippet, a user is fetched from a <code>ClickHouse</code> database
+using five different queries. In the "BAD" cases the query is built directly from user-controlled data.
+In the "GOOD" case the user-controlled data is safely embedded into the query by using query parameters.
 </p>
 
 <p>

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

@@ -0,0 +1,5 @@
+| ClickHouseDriver.py:15:22:15:106 | ControlFlowNode for Attribute() | ClickHouseDriver.py:15:52:15:105 | ControlFlowNode for BinaryExpr |
+| ClickHouseDriver.py:18:5:18:87 | ControlFlowNode for Attribute() | ClickHouseDriver.py:18:33:18:86 | ControlFlowNode for BinaryExpr |
+| ClickHouseDriver.py:22:5:22:62 | ControlFlowNode for Attribute() | ClickHouseDriver.py:22:33:22:37 | ControlFlowNode for query |
+| ClickHouseDriver.py:27:5:27:74 | ControlFlowNode for Attribute() | ClickHouseDriver.py:27:20:27:73 | ControlFlowNode for BinaryExpr |
+| 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

@@ -0,0 +1,32 @@
+from django.conf.urls import url
+from clickhouse_driver import Client
+from clickhouse_driver import connect
+from aioch import Client as aiochClient
+
+# Dummy Client subclass
+class MyClient(Client):
+    def dummy(self):
+        return None
+
+def show_user(request, username):
+
+    # BAD -- Untrusted user input is directly injected into the sql query using async library 'aioch'
+    aclient = aiochClient("localhost")
+    progress = await aclient.execute_with_progress("SELECT * FROM users WHERE username = '%s'" % username)
+
+    # BAD -- Untrusted user input is directly injected into the sql query using native client of library 'clickhouse_driver'
+    Client('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
+
+    # GOOD -- query uses prepared statements
+    query = "SELECT * FROM users WHERE username = %(username)s"
+    Client('localhost').execute(query, {"username": username})
+
+    # BAD -- Untrusted user input is directly injected into the sql query using PEP249 interface
+    conn = connect('clickhouse://localhost')
+    cursor = conn.cursor()
+    cursor.execute("SELECT * FROM users WHERE username = '%s'" % username)
+
+    # BAD -- Untrusted user input is directly injected into the sql query using MyClient, which is a subclass of Client
+    MyClient('localhost').execute("SELECT * FROM users WHERE username = '%s'" % username)
+
+urlpatterns = [url(r'^users/(?P<username>[^/]+)$', show_user)]

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

@@ -0,0 +1,6 @@
+import python
+import experimental.semmle.python.frameworks.ClickHouseDriver
+import semmle.python.Concepts
+
+from SqlExecution::Range s
+select s, s.getSql()