From eb6c2b96b9e3e96d119997c9ec8316dba168ea75 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 6 Aug 2026 10:22:35 -0700 Subject: [PATCH] Fix for SQL injection issue in table filters, refs #2868 --- datasette/filters.py | 58 ++++++++++++++++++++++++----------------- tests/test_filters.py | 33 +++++++++++++++++++++-- tests/test_table_api.py | 30 ++++++++++++++++++++- 3 files changed, 94 insertions(+), 27 deletions(-) diff --git a/datasette/filters.py b/datasette/filters.py index 1d4e32c2..3cfb36e5 100644 --- a/datasette/filters.py +++ b/datasette/filters.py @@ -209,10 +209,14 @@ class TemplatedFilter(Filter): if self.numeric and converted.isdigit(): converted = int(converted) if self.no_argument: - kwargs = {"c": column} + kwargs = {"c": _quote_sqlite_identifier(column)} converted = None else: - kwargs = {"c": column, "p": f"p{param_counter}", "t": table} + kwargs = { + "c": _quote_sqlite_identifier(column), + "p": f"p{param_counter}", + "t": _quote_sqlite_identifier(table), + } return self.sql_template.format(**kwargs), converted def human_clause(self, column, value): @@ -226,6 +230,14 @@ class TemplatedFilter(Filter): return template.format(c=column, v=value) +def _quote_sqlite_identifier(identifier): + # Preserve the historic always-quoted SQL generated by TemplatedFilter. + escaped = escape_sqlite(identifier) + if escaped == identifier: + return f'"{identifier}"' + return escaped + + class InFilter(Filter): key = "in" display = "in" @@ -267,56 +279,56 @@ class Filters: TemplatedFilter( "exact", "=", - '"{c}" = :{p}', + "{c} = :{p}", lambda c, v: "{c} = {v}" if v.isdigit() else '{c} = "{v}"', ), TemplatedFilter( "not", "!=", - '"{c}" != :{p}', + "{c} != :{p}", lambda c, v: "{c} != {v}" if v.isdigit() else '{c} != "{v}"', ), TemplatedFilter( "contains", "contains", - '"{c}" like :{p}', + "{c} like :{p}", '{c} contains "{v}"', format="%{}%", ), TemplatedFilter( "notcontains", "does not contain", - '"{c}" not like :{p}', + "{c} not like :{p}", '{c} does not contain "{v}"', format="%{}%", ), TemplatedFilter( "endswith", "ends with", - '"{c}" like :{p}', + "{c} like :{p}", '{c} ends with "{v}"', format="%{}", ), TemplatedFilter( "startswith", "starts with", - '"{c}" like :{p}', + "{c} like :{p}", '{c} starts with "{v}"', format="{}%", ), - TemplatedFilter("gt", ">", '"{c}" > :{p}', "{c} > {v}", numeric=True), + TemplatedFilter("gt", ">", "{c} > :{p}", "{c} > {v}", numeric=True), TemplatedFilter( - "gte", "\u2265", '"{c}" >= :{p}', "{c} \u2265 {v}", numeric=True + "gte", "\u2265", "{c} >= :{p}", "{c} \u2265 {v}", numeric=True ), - TemplatedFilter("lt", "<", '"{c}" < :{p}', "{c} < {v}", numeric=True), + TemplatedFilter("lt", "<", "{c} < :{p}", "{c} < {v}", numeric=True), TemplatedFilter( - "lte", "\u2264", '"{c}" <= :{p}', "{c} \u2264 {v}", numeric=True + "lte", "\u2264", "{c} <= :{p}", "{c} \u2264 {v}", numeric=True ), - TemplatedFilter("like", "like", '"{c}" like :{p}', '{c} like "{v}"'), + TemplatedFilter("like", "like", "{c} like :{p}", '{c} like "{v}"'), TemplatedFilter( - "notlike", "not like", '"{c}" not like :{p}', '{c} not like "{v}"' + "notlike", "not like", "{c} not like :{p}", '{c} not like "{v}"' ), - TemplatedFilter("glob", "glob", '"{c}" glob :{p}', '{c} glob "{v}"'), + TemplatedFilter("glob", "glob", "{c} glob :{p}", '{c} glob "{v}"'), InFilter(), NotInFilter(), ] @@ -325,13 +337,13 @@ class Filters: TemplatedFilter( "arraycontains", "array contains", - """:{p} in (select value from json_each([{t}].[{c}]))""", + """:{p} in (select value from json_each({t}.{c}))""", '{c} contains "{v}"', ), TemplatedFilter( "arraynotcontains", "array does not contain", - """:{p} not in (select value from json_each([{t}].[{c}]))""", + """:{p} not in (select value from json_each({t}.{c}))""", '{c} does not contain "{v}"', ), ] @@ -339,30 +351,28 @@ class Filters: else [] ) + [ + TemplatedFilter("date", "date", "date({c}) = :{p}", '"{c}" is on date {v}'), TemplatedFilter( - "date", "date", 'date("{c}") = :{p}', '"{c}" is on date {v}' - ), - TemplatedFilter( - "isnull", "is null", '"{c}" is null', "{c} is null", no_argument=True + "isnull", "is null", "{c} is null", "{c} is null", no_argument=True ), TemplatedFilter( "notnull", "is not null", - '"{c}" is not null', + "{c} is not null", "{c} is not null", no_argument=True, ), TemplatedFilter( "isblank", "is blank", - '("{c}" is null or "{c}" = "")', + "({c} is null or {c} = '')", "{c} is blank", no_argument=True, ), TemplatedFilter( "notblank", "is not blank", - '("{c}" is not null and "{c}" != "")', + "({c} is not null and {c} != '')", "{c} is not blank", no_argument=True, ), diff --git a/tests/test_filters.py b/tests/test_filters.py index 8d0f3512..9f201fdf 100644 --- a/tests/test_filters.py +++ b/tests/test_filters.py @@ -66,12 +66,12 @@ from datasette.utils.asgi import Request # JSON arraycontains, arraynotcontains ( (("Availability+Info__arraycontains", "yes"),), - [":p0 in (select value from json_each([table].[Availability+Info]))"], + [':p0 in (select value from json_each("table"."Availability+Info"))'], ["yes"], ), ( (("Availability+Info__arraynotcontains", "yes"),), - [":p0 not in (select value from json_each([table].[Availability+Info]))"], + [':p0 not in (select value from json_each("table"."Availability+Info"))'], ["yes"], ), ], @@ -83,6 +83,35 @@ def test_build_where(args, expected_where, expected_params): assert {f"p{i}": param for i, param in enumerate(expected_params)} == actual_params +@pytest.mark.parametrize( + "key,expected_where", + ( + ( + 'has"quote__exact', + '"has""quote" = :p0', + ), + ( + 'has"quote__isnull', + '"has""quote" is null', + ), + ( + "has]bracket__arraycontains", + ':p0 in (select value from json_each("table"."has]bracket"))', + ), + ), +) +def test_build_where_escapes_column_names(key, expected_where): + filters = Filters(((key, "value"),)) + sql_bits, _ = filters.build_where_clauses("table") + assert sql_bits == [expected_where] + + +def test_build_where_escapes_table_name(): + filters = Filters((("tags__arraycontains", "value"),)) + sql_bits, _ = filters.build_where_clauses("items]bracket") + assert sql_bits == [':p0 in (select value from json_each("items]bracket"."tags"))'] + + @pytest.mark.asyncio async def test_through_filters_from_request(ds_client): request = Request.fake( diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 32dd37f2..6c0c021b 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -4,7 +4,7 @@ import urllib import pytest from datasette.fixtures import generate_compound_rows, generate_sortable_rows -from datasette.utils import detect_json1 +from datasette.utils import detect_json1, tilde_encode from datasette.utils.sqlite import sqlite_version from .fixtures import make_app_client @@ -689,6 +689,34 @@ async def test_table_filter_queries_multiple_of_same_type(ds_client): ] == response.json()["rows"] +@pytest.mark.skipif(not detect_json1(), reason="Requires the SQLite json1 module") +def test_table_filters_quote_identifiers(): + with make_app_client( + extra_databases={"demo.db": """ + create table "items]bracket" ( + id integer primary key, + "name""quote" text, + "tags]bracket" text + ); + insert into "items]bracket" values (1, 'Alice', '["red"]'); + """}, + ) as client: + table_path = tilde_encode("items]bracket") + exact_query = urllib.parse.urlencode( + {'name"quote__exact': "Alice", "_shape": "arrays"} + ) + exact_response = client.get(f"/demo/{table_path}.json?{exact_query}") + assert exact_response.status == 200 + assert exact_response.json["rows"] == [[1, "Alice", '["red"]']] + + array_query = urllib.parse.urlencode( + {"tags]bracket__arraycontains": "red", "_shape": "arrays"} + ) + array_response = client.get(f"/demo/{table_path}.json?{array_query}") + assert array_response.status == 200 + assert array_response.json["rows"] == [[1, "Alice", '["red"]']] + + @pytest.mark.skipif(not detect_json1(), reason="Requires the SQLite json1 module") @pytest.mark.asyncio async def test_table_filter_json_arraycontains(ds_client):