Summary
eql_v2.sort_compare(id_column, val_column, tbl, direction, filter) (in src/operators/sort.sql:475) concatenates the raw filter text into a dynamically-built query at line 498:
```sql
IF filter IS NOT NULL THEN
query := query || ' WHERE ' || filter;
END IF;
```
The constructed query is then handed to eql_v2.order_by_compare(query, direction) (src/operators/order_by.sql), which executes it via EXECUTE format(... %s, query). Two layers of dynamic SQL, with the user-supplied filter interpolated raw at the inner layer.
The function is not SECURITY DEFINER, so an attacker calling it directly only escalates within their own session. The risk is when an application forwards untrusted input (e.g. from a request parameter) into filter — which is exactly what its @warning says not to do, but it's the kind of footgun worth removing.
Reproducer
```sql
-- Anything past the WHERE clause executes as written
SELECT * FROM eql_v2.sort_compare('id', 'e', 'users', 'ASC',
'1=1; CREATE TABLE pwned(x int)--');
```
Mitigation options
In rough order of effort:
-
Remove the convenience overload entirely and require callers to pre-build the rows array and use the sort_compare(bigint[], eql_v2_encrypted[], text) overload, which doesn't accept arbitrary SQL. Forces callers to explicitly opt into dynamic SQL.
-
Restrict filter to a structured, non-textual form — e.g. accept a jsonb filter spec describing column/operator/value tuples, then build the WHERE clause from it using format('%I %s %L', ...). More work but eliminates the injection vector.
-
Move the body into a SECURITY DEFINER function with a strict allowlist for what filter can contain (regex check for safe characters/keywords). Cheaper but still relies on a denylist that's hard to get right.
-
REVOKE EXECUTE FROM PUBLIC on the convenience overload and document it as internal/server-side-only. Doesn't fix the vulnerability but limits who can reach it. This is the minimum-risk shipping option and could go in alongside any of (1)-(3).
Notes
Related
Summary
eql_v2.sort_compare(id_column, val_column, tbl, direction, filter)(insrc/operators/sort.sql:475) concatenates the rawfiltertext into a dynamically-built query at line 498:```sql
IF filter IS NOT NULL THEN
query := query || ' WHERE ' || filter;
END IF;
```
The constructed query is then handed to
eql_v2.order_by_compare(query, direction)(src/operators/order_by.sql), which executes it viaEXECUTE format(... %s, query). Two layers of dynamic SQL, with the user-suppliedfilterinterpolated raw at the inner layer.The function is not SECURITY DEFINER, so an attacker calling it directly only escalates within their own session. The risk is when an application forwards untrusted input (e.g. from a request parameter) into
filter— which is exactly what its@warningsays not to do, but it's the kind of footgun worth removing.Reproducer
```sql
-- Anything past the WHERE clause executes as written
SELECT * FROM eql_v2.sort_compare('id', 'e', 'users', 'ASC',
'1=1; CREATE TABLE pwned(x int)--');
```
Mitigation options
In rough order of effort:
Remove the convenience overload entirely and require callers to pre-build the rows array and use the
sort_compare(bigint[], eql_v2_encrypted[], text)overload, which doesn't accept arbitrary SQL. Forces callers to explicitly opt into dynamic SQL.Restrict
filterto a structured, non-textual form — e.g. accept ajsonbfilter spec describing column/operator/value tuples, then build the WHERE clause from it usingformat('%I %s %L', ...). More work but eliminates the injection vector.Move the body into a SECURITY DEFINER function with a strict allowlist for what
filtercan contain (regex check for safe characters/keywords). Cheaper but still relies on a denylist that's hard to get right.REVOKE EXECUTE FROM PUBLIC on the convenience overload and document it as internal/server-side-only. Doesn't fix the vulnerability but limits who can reach it. This is the minimum-risk shipping option and could go in alongside any of (1)-(3).
Notes
main— fix: pin search_path on every eql_v2 function #177 added aSET search_pathclause to the function but did not change the body.@warningDoxygen comment already says "Use only with trusted input." That's a documented contract, but the function is reachable fromPUBLICby default.Related
src/operators/sort.sql:475-505— the convenience overloadsrc/operators/order_by.sql— the innerEXECUTEconsumer