[#1725] datastore_search and _delete don't work with multiple statements

What we're trying to avoid is for an insecure extension to allow a malicious
user to create queries like:

```
SELECT * FROM "foo" WHERE (1=1); DELETE FROM "foo"; -- AND "bar"='5');
```

This doesn't avoid all possible SQL injection vectors, but it's one less issue
to worry about.

ckanext/datastore/db.py

@@ -804,15 +804,13 @@ def delete_data(context, data_dict):
                                              field_ids, query_dict)
 
     where_clause, where_values = _where(query_dict['where'])
-
-    context['connection'].execute(
-        u'DELETE FROM "{0}" {1}'.format(
-            data_dict['resource_id'],
-            where_clause
-        ),
-        where_values
+    sql_string = u'DELETE FROM "{0}" {1}'.format(
+        data_dict['resource_id'],
+        where_clause
     )
 
+    _execute_single_statement(context, sql_string, where_values)
+
 
 def validate_query(context, data_dict):
     all_fields = _get_fields(context, data_dict)
@@ -895,14 +893,25 @@ def search_data(context, data_dict):
         sort=sort_clause,
         limit=limit,
         offset=offset)
-    sql_string = sql_string.replace('%', '%%')
-    results = context['connection'].execute(
-        sql_string.format(where=where_clause), [where_values])
+    sql_string = sql_string.replace('%', '%%').format(where=where_clause)
+
+    results = _execute_single_statement(context, sql_string, where_values)
 
     _insert_links(data_dict, limit, offset)
     return format_results(context, results, data_dict)
 
 
+def _execute_single_statement(context, sql_string, where_values):
+    if not datastore_helpers.is_single_statement(sql_string):
+        raise ValidationError({
+            'query': ['Query is not a single statement.']
+        })
+
+    results = context['connection'].execute(sql_string, [where_values])
+
+    return results
+
+
 def format_results(context, results, data_dict):
     result_fields = []
     for field in results.cursor.description:

ckanext/datastore/tests/sample_datastore_plugin.py

@@ -7,7 +7,7 @@ class SampleDataStorePlugin(p.SingletonPlugin):
     p.implements(interfaces.IDatastore, inherit=True)
 
     def datastore_validate_query(self, context, data_dict, all_field_ids):
-        valid_filters = ('age_between', 'age_not_between')
+        valid_filters = ('age_between', 'age_not_between', 'insecure_filter')
         filters = data_dict.get('filters', {})
         for key in filters.keys():
             if key in valid_filters:
@@ -39,5 +39,10 @@ def _where(self, data_dict):
             clause = ('"age" < %s OR "age" > %s',
                       age_not_between[0], age_not_between[1])
             where_clauses.append(clause)
+        if 'insecure_filter' in filters:
+            insecure_filter = filters['insecure_filter']
+
+            clause = (insecure_filter,)
+            where_clauses.append(clause)
 
         return where_clauses

ckanext/datastore/tests/test_interface.py

@@ -5,6 +5,7 @@
 import ckan.new_tests.factories as factories
 
 assert_equals = nose.tools.assert_equals
+assert_raises = nose.tools.assert_raises
 
 
 class TestInterfaces(object):
@@ -72,6 +73,26 @@ def test_datastore_search_custom_filters_have_the_correct_operator_precedence(se
         assert result['records'][0]['age'] == 30, result
         assert_equals(result['filters'], filters)
 
+    def test_datastore_search_insecure_filter(self):
+        records = [
+            {'age': 20}, {'age': 30}, {'age': 40}
+        ]
+        resource = self._create_datastore_resource(records)
+        sql_inject = '1=1); DELETE FROM "%s"; COMMIT; SELECT * FROM "%s";--' \
+                     % (resource['id'], resource['id'])
+        filters = {
+            'insecure_filter': sql_inject
+        }
+
+        assert_raises(p.toolkit.ValidationError,
+                      helpers.call_action, 'datastore_search',
+                      resource_id=resource['id'], filters=filters)
+
+        result = helpers.call_action('datastore_search',
+                                     resource_id=resource['id'])
+
+        assert result['total'] == 3, result
+
     def test_datastore_delete_can_create_custom_filters(self):
         records = [
             {'age': 20}, {'age': 30}, {'age': 40}
@@ -119,6 +140,28 @@ def test_datastore_delete_custom_filters_have_the_correct_operator_precedence(se
         new_records_ages.sort()
         assert_equals(new_records_ages, [20, 40])
 
+    def test_datastore_delete_insecure_filter(self):
+        records = [
+            {'age': 20}, {'age': 30}, {'age': 40}
+        ]
+        resource = self._create_datastore_resource(records)
+        sql_inject = '1=1); DELETE FROM "%s"; SELECT * FROM "%s";--' \
+                     % (resource['id'], resource['id'])
+        filters = {
+            'age': 20,
+            'insecure_filter': sql_inject
+        }
+
+        assert_raises(p.toolkit.ValidationError,
+                      helpers.call_action, 'datastore_delete',
+                      resource_id=resource['id'], force=True,
+                      filters=filters)
+
+        result = helpers.call_action('datastore_search',
+                                     resource_id=resource['id'])
+
+        assert result['total'] == 3, result
+
     def _create_datastore_resource(self, records):
         dataset = factories.Dataset()
         resource = factories.Resource(package=dataset)