Fix for SQL injection issue in table filters, refs #2868

This commit is contained in:
Simon Willison 2026-08-06 10:22:35 -07:00
commit eb6c2b96b9
3 changed files with 94 additions and 27 deletions

View file

@ -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,
),

View file

@ -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(

View file

@ -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):