Skip to content

persistence: a _-prefixed conditional criterion matches every resource on SQLite (deletes a non-matching one); parameter names reach SQL unescaped #866

Description

@aacruzgon

Summary

Two defects in how a conditional criterion's name reaches the SQLite query builder. The first deletes a resource that does not match the criteria; the second passes the name into SQL unescaped.

Needs a security review before deciding on public detail; this report deliberately stops at the observable symptoms.

1. A _-prefixed name matches every resource of the type (SQLite only)

Reproduced on the batch_conformance harness: with exactly one Patient in the tenant, batch DELETE Patient?_id:exact=nonexistent answered 204 No Content and the Patient was gone. With two or more Patients the same request answers 412 (multiple matches), so this is not mass deletion, but it removes a resource the criteria did not name. DELETE /Patient?_id:exact=x on the resource endpoint takes the same path; this predates #511.

Cause: sqlite/search/query_builder.rs:405-413 routes any name starting with _ (except _tag/_profile/_security/_source/_language) to build_special_parameter_condition, whose fallback arm (:576-578) returns None with an in-code comment that this "drops the parameter and the search answers with every resource of the type"; build (:237-242) skips a None condition. #535 item 2 named this "the #474 failure mode" when _language hit it; the arm is still reachable for any other _ name, and conditional criteria are the write-path way to reach it. PostgreSQL does not share this: postgres/search/query_builder.rs:637-656 falls through to a typed condition that matches nothing.

Fix: a dropped condition must never mean "match everything" on a conditional interaction. Reject unknown _ names (and any name with a modifier the special path does not understand) with 400; the ordinary search path should get the same treatment per #474.

2. The parameter name is interpolated into SQL unescaped

param.name is formatted into SQL at sqlite/search/query_builder.rs:320, 461, 503 and postgres/search/query_builder.rs:482, 612, 799, 835, unlike the neighbouring compartment builder (sqlite/…:358) which escapes '. Reproduced: batch DELETE Patient?ident'ifier=x answers 500 Internal Server Error (a database syntax error surfaces). Conditional criteria names are client-controlled query-string keys. Whether this is exploitable beyond a 500 was not explored. Fix: validate names against [A-Za-z0-9_\-]+(:[a-z\-]+)? before they reach a builder, and bind or escape at the builder regardless.

Found while validating #511 (see PR #860).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions