Fix for security issue #918

simonw · Aug 9, 2020 · 7f10f0f · 7f10f0f
1 parent de90b75
commit 7f10f0f
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 1 deletion.
diff --git a/datasette/templates/query.html b/datasette/templates/query.html
@@ -52,7 +52,7 @@ <h3>Query parameters</h3>
     {% endif %}
     <p>
         <button id="sql-format" type="button" hidden>Format SQL</button>
-        {% if canned_query %}<input type="hidden" name="csrftoken" value="{{ csrftoken() }}">{% endif %}
+        {% if canned_write %}<input type="hidden" name="csrftoken" value="{{ csrftoken() }}">{% endif %}
         <input type="submit" value="Run SQL">
     </p>
 </form>

diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py
@@ -12,6 +12,7 @@ def canned_write_client():
             "databases": {
                 "data": {
                     "queries": {
+                        "canned_read": {"sql": "select * from names"},
                         "add_name": {
                             "sql": "insert into names (name) values (:name)",
                             "write": True,
@@ -69,6 +70,22 @@ def test_insert(canned_write_client):
     assert [["Query executed, 1 row affected", 1]] == messages
 
 
+@pytest.mark.parametrize(
+    "query_name,expect_csrf_hidden_field",
+    [("canned_read", False), ("add_name_specify_id", True), ("add_name", True),],
+)
+def test_canned_query_form_csrf_hidden_field(
+    canned_write_client, query_name, expect_csrf_hidden_field
+):
+    response = canned_write_client.get("/data/{}".format(query_name))
+    html = response.text
+    fragment = '<input type="hidden" name="csrftoken" value="'
+    if expect_csrf_hidden_field:
+        assert fragment in html
+    else:
+        assert fragment not in html
+
+
 def test_insert_with_cookies_requires_csrf(canned_write_client):
     response = canned_write_client.post(
         "/data/add_name",
@@ -148,6 +165,7 @@ def test_canned_query_permissions_on_database_page(canned_write_client):
         q["name"] for q in canned_write_client.get("/data.json").json["queries"]
     }
     assert {
+        "canned_read",
         "add_name",
         "add_name_specify_id",
         "update_name",
@@ -164,6 +182,7 @@ def test_canned_query_permissions_on_database_page(canned_write_client):
     assert [
         {"name": "add_name", "private": False},
         {"name": "add_name_specify_id", "private": False},
+        {"name": "canned_read", "private": False},
         {"name": "delete_name", "private": True},
         {"name": "from_async_hook", "private": False},
         {"name": "from_hook", "private": False},