SQL injection: NostrFilter.to_sql_components interpolates client filter values into SQL #8

New issue

Open

opened 2026-10-09 16:47:12 +00:00 by padreug · 0 comments

padreug commented

2026-10-09 16:47:12 +00:00

Owner

relay/filter.py builds the events query by f-string-interpolating client-controlled filter values, wrapped in single quotes, with no escaping and no parameter binding:

relay/filter.py:108-109 — ids = ",".join([f"'{_id}'" for _id in self.ids]) / where.append(f"id IN ({ids})")
relay/filter.py:112-113 — same for authors → pubkey IN (...)
relay/filter.py:81,89,97 — same for #e, #p, #d tag values

ids, authors, e, p, d are list[str] with no hex validation. Only since/until are bound (:120-126); relay_id is bound but the injected text sits in the query string, so binding does not protect it.

Reachable without authentication: require_auth_filter defaults to false, so REQ → client_connection._handle_message (:138-151) → _handle_request (:324) → crud.get_events (crud.py:111) → to_sql_components. A frame like ["REQ","s",{"ids":["x') UNION SELECT ... --"]}] defeats the relay_id = :relay_id tenant scope (cross-relay reads of every relay's events). The same function feeds crud.mark_events_deleted (crud.py:184) and crud.delete_events (crud.py:195), so the sink is also reached from DELETE/UPDATE paths:

addressable-event replacement, relay/client_connection.py:196-206 — the incoming event's own d tag goes into #d;
NIP-09 a-tag deletion, relay/client_connection.py:268-286 — _address_filter checks kind and pubkey == author but passes d_tag through unvalidated (:273, :284).

Those two routes need a signed event, but any key works, so they are effectively unauthenticated too.

Impact: cross-tenant read of all events on a multi-relay deployment; driver-dependent over-deletion via the delete paths.

Fix direction: bind every list element as a named parameter (:ids_0, :ids_1, ...) and put the values in the values dict; do the same for kinds for consistency. (inner_joins, where, values) contract stays, so the three callers in crud.py do not change. Hex validation on ids/authors/#e/#p (64 lowercase hex) as defence in depth. The sandbox fix PR (sandbox nostrrelay#13, bind_in helper + tests/test_filter_sqli.py) applies to our relay/filter.py with a four-line offset. Land before real #2 (generic single-letter tag filters) rewrites the same function.

Found during reforge run #1 (sandbox nostrrelay#2, nostrrelay#11).

`relay/filter.py` builds the events query by f-string-interpolating client-controlled filter values, wrapped in single quotes, with no escaping and no parameter binding: - `relay/filter.py:108-109` — `ids = ",".join([f"'{_id}'" for _id in self.ids])` / `where.append(f"id IN ({ids})")` - `relay/filter.py:112-113` — same for `authors` → `pubkey IN (...)` - `relay/filter.py:81,89,97` — same for `#e`, `#p`, `#d` tag values `ids`, `authors`, `e`, `p`, `d` are `list[str]` with no hex validation. Only `since`/`until` are bound (`:120-126`); `relay_id` is bound but the injected text sits in the query string, so binding does not protect it. Reachable without authentication: `require_auth_filter` defaults to false, so `REQ` → `client_connection._handle_message` (`:138-151`) → `_handle_request` (`:324`) → `crud.get_events` (`crud.py:111`) → `to_sql_components`. A frame like `["REQ","s",{"ids":["x') UNION SELECT ... --"]}]` defeats the `relay_id = :relay_id` tenant scope (cross-relay reads of every relay's events). The same function feeds `crud.mark_events_deleted` (`crud.py:184`) and `crud.delete_events` (`crud.py:195`), so the sink is also reached from DELETE/UPDATE paths: - addressable-event replacement, `relay/client_connection.py:196-206` — the incoming event's own `d` tag goes into `#d`; - NIP-09 `a`-tag deletion, `relay/client_connection.py:268-286` — `_address_filter` checks `kind` and `pubkey == author` but passes `d_tag` through unvalidated (`:273`, `:284`). Those two routes need a signed event, but any key works, so they are effectively unauthenticated too. Impact: cross-tenant read of all events on a multi-relay deployment; driver-dependent over-deletion via the delete paths. Fix direction: bind every list element as a named parameter (`:ids_0, :ids_1, ...`) and put the values in the `values` dict; do the same for `kinds` for consistency. `(inner_joins, where, values)` contract stays, so the three callers in `crud.py` do not change. Hex validation on `ids`/`authors`/`#e`/`#p` (64 lowercase hex) as defence in depth. The sandbox fix PR (sandbox nostrrelay#13, `bind_in` helper + `tests/test_filter_sqli.py`) applies to our `relay/filter.py` with a four-line offset. Land before real #2 (generic single-letter tag filters) rewrites the same function. Found during reforge run #1 (sandbox nostrrelay#2, nostrrelay#11).

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

aiolabs/nostrrelay#8

No description provided.

Rows
Columns