From ba4fc5c385ad9677801557ce9f17e77204f92c9a Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 13:40:05 +1300 Subject: [PATCH 1/6] feat: exposed arbitrary sql --- bin/installcheck | 2 +- ...31_anon_dynamic_sql_function_executable.md | 64 ++++ ...ticated_dynamic_sql_function_executable.md | 64 ++++ ...1_anon_dynamic_sql_function_executable.sql | 161 +++++++++ ...icated_dynamic_sql_function_executable.sql | 161 +++++++++ mkdocs.yaml | 2 + splinter.sql | 324 +++++++++++++++++- ...1_anon_dynamic_sql_function_executable.out | 186 ++++++++++ ...icated_dynamic_sql_function_executable.out | 186 ++++++++++ test/expected/queries_are_unionable.out | 6 +- ...1_anon_dynamic_sql_function_executable.sql | 166 +++++++++ ...icated_dynamic_sql_function_executable.sql | 166 +++++++++ test/sql/queries_are_unionable.sql | 6 +- 13 files changed, 1490 insertions(+), 4 deletions(-) create mode 100644 docs/0031_anon_dynamic_sql_function_executable.md create mode 100644 docs/0032_authenticated_dynamic_sql_function_executable.md create mode 100644 lints/0031_anon_dynamic_sql_function_executable.sql create mode 100644 lints/0032_authenticated_dynamic_sql_function_executable.sql create mode 100644 test/expected/0031_anon_dynamic_sql_function_executable.out create mode 100644 test/expected/0032_authenticated_dynamic_sql_function_executable.out create mode 100644 test/sql/0031_anon_dynamic_sql_function_executable.sql create mode 100644 test/sql/0032_authenticated_dynamic_sql_function_executable.sql diff --git a/bin/installcheck b/bin/installcheck index 659aa2fb..3a74aa70 100755 --- a/bin/installcheck +++ b/bin/installcheck @@ -52,7 +52,7 @@ else fi # Execute the test fixtures -psql -v ON_ERROR_STOP= -f test/fixtures.sql -f lints/0001*.sql -f lints/0002*.sql -f lints/0003*.sql -f lints/0004*.sql -f lints/0005*.sql -f lints/0006*.sql -f lints/0007*.sql -f lints/0008*.sql -f lints/0009*.sql -f lints/0010*.sql -f lints/0011*.sql -f lints/0013*.sql -f lints/0014*.sql -f lints/0015*.sql -f lints/0016*.sql -f lints/0017*.sql -f lints/0018*.sql -f lints/0019*.sql -f lints/0020*.sql -f lints/0021*.sql -f lints/0022*.sql -f lints/0023*.sql -f lints/0024*.sql -f lints/0025*.sql -f lints/0026*.sql -f lints/0027*.sql -f lints/0028*.sql -f lints/0029*.sql -f lints/0030*.sql -d contrib_regression +psql -v ON_ERROR_STOP= -f test/fixtures.sql -f lints/0001*.sql -f lints/0002*.sql -f lints/0003*.sql -f lints/0004*.sql -f lints/0005*.sql -f lints/0006*.sql -f lints/0007*.sql -f lints/0008*.sql -f lints/0009*.sql -f lints/0010*.sql -f lints/0011*.sql -f lints/0013*.sql -f lints/0014*.sql -f lints/0015*.sql -f lints/0016*.sql -f lints/0017*.sql -f lints/0018*.sql -f lints/0019*.sql -f lints/0020*.sql -f lints/0021*.sql -f lints/0022*.sql -f lints/0023*.sql -f lints/0024*.sql -f lints/0025*.sql -f lints/0026*.sql -f lints/0027*.sql -f lints/0028*.sql -f lints/0029*.sql -f lints/0030*.sql -f lints/0031*.sql -f lints/0032*.sql -d contrib_regression # Run tests ${REGRESS} --use-existing --dbname=contrib_regression --inputdir=${TESTDIR} ${TESTS} diff --git a/docs/0031_anon_dynamic_sql_function_executable.md b/docs/0031_anon_dynamic_sql_function_executable.md new file mode 100644 index 00000000..6d25b9db --- /dev/null +++ b/docs/0031_anon_dynamic_sql_function_executable.md @@ -0,0 +1,64 @@ +**Level:** ERROR / WARN + +**Summary:** This function is callable without signing in and appears to `EXECUTE` a SQL string passed in by the caller. + +**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so anyone with the public anon key can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS (level `ERROR`). If it is `SECURITY INVOKER` the SQL runs as `anon` and is limited by that role's grants and RLS (level `WARN`). Either way the caller can read your schema and any data `anon` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. + +> **See also: lint [0032_authenticated_dynamic_sql_function_executable](0032_authenticated_dynamic_sql_function_executable.md).** It is the same check for signed-in users. Revoking from `anon` alone usually leaves the function callable by every signed-up user, so address both together. Lint [0028_anon_security_definer_function_executable](0028_anon_security_definer_function_executable.md) covers `SECURITY DEFINER` functions exposed to `anon` in general. + +--- + +### Rationale + +Helpers like `exec_sql(query text)` or `run_query(sql text)` let a client send SQL to the database and get the result back. Postgres grants `EXECUTE` on new functions to `PUBLIC`, and Supabase also grants it to `anon` and `authenticated`, so a function like this is callable by clients unless `EXECUTE` is revoked. Checking that the string starts with `select` does not make it safe. + +This lint reads the function body, and only for PL/pgSQL functions in exposed schemas. A function is flagged when: + +1. a `text` or `varchar` parameter is used inside an `EXECUTE` statement, or +2. a `text` or `varchar` parameter is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. + +Parameters bound with `EXECUTE ... USING` and parameters wrapped in `quote_ident()`, `quote_literal()` or `quote_nullable()` are not counted. + +### How to Resolve + +**Option 1: Revoke `EXECUTE`** + +```sql +revoke execute on function public.exec_sql(text) from anon, public; +``` + +**Option 2: Drop the function** + +```sql +drop function public.exec_sql(text); +``` + +**Option 3: Replace it with fixed operations** + +Expose one function per operation with typed arguments and no string-built SQL. If you have to build SQL, pass values with `EXECUTE ... USING`, quote identifiers with `quote_ident()`, and check table and column names against an allow-list. + +### Example + +Given this function: + +```sql +create function public.exec_sql(query text) returns json +language plpgsql security definer as $$ +declare r json; +begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; +end $$; +``` + +Fix: + +```sql +revoke execute on function public.exec_sql(text) from anon, authenticated, public; +``` + +### False Positives + +- A `text` parameter used as an identifier through `format('%I', param)` is safe but can still be flagged. Use `quote_ident(param)` instead. +- A function that checks the string against a strict allow-list before running it. Confirm the allow-list is complete, then suppress the finding. +- Only PL/pgSQL bodies are inspected. Functions in other languages, or that build SQL in a helper function, are not detected. diff --git a/docs/0032_authenticated_dynamic_sql_function_executable.md b/docs/0032_authenticated_dynamic_sql_function_executable.md new file mode 100644 index 00000000..583b9600 --- /dev/null +++ b/docs/0032_authenticated_dynamic_sql_function_executable.md @@ -0,0 +1,64 @@ +**Level:** ERROR / WARN + +**Summary:** This function is callable by signed-in users and appears to `EXECUTE` a SQL string passed in by the caller. + +**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so any signed-in user (with open signup, anyone with a throwaway account) can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS (level `ERROR`). If it is `SECURITY INVOKER` the SQL runs as `authenticated` and is limited by that role's grants and RLS (level `WARN`). Either way the caller can read your schema and any data `authenticated` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. + +> **See also: lint [0031_anon_dynamic_sql_function_executable](0031_anon_dynamic_sql_function_executable.md).** It is the same check for `anon`. Revoking from `authenticated` alone can leave the function callable without signing in, so address both together. Lint [0029_authenticated_security_definer_function_executable](0029_authenticated_security_definer_function_executable.md) covers `SECURITY DEFINER` functions exposed to `authenticated` in general. + +--- + +### Rationale + +Helpers like `exec_sql(query text)` or `run_query(sql text)` let a client send SQL to the database and get the result back. Postgres grants `EXECUTE` on new functions to `PUBLIC`, and Supabase also grants it to `anon` and `authenticated`, so a function like this is callable by clients unless `EXECUTE` is revoked. Checking that the string starts with `select` does not make it safe. + +This lint reads the function body, and only for PL/pgSQL functions in exposed schemas. A function is flagged when: + +1. a `text` or `varchar` parameter is used inside an `EXECUTE` statement, or +2. a `text` or `varchar` parameter is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. + +Parameters bound with `EXECUTE ... USING` and parameters wrapped in `quote_ident()`, `quote_literal()` or `quote_nullable()` are not counted. + +### How to Resolve + +**Option 1: Revoke `EXECUTE`** + +```sql +revoke execute on function public.exec_sql(text) from authenticated, public; +``` + +**Option 2: Drop the function** + +```sql +drop function public.exec_sql(text); +``` + +**Option 3: Replace it with fixed operations** + +Expose one function per operation with typed arguments and no string-built SQL. If you have to build SQL, pass values with `EXECUTE ... USING`, quote identifiers with `quote_ident()`, and check table and column names against an allow-list. + +### Example + +Given this function: + +```sql +create function public.exec_sql(query text) returns json +language plpgsql security definer as $$ +declare r json; +begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; +end $$; +``` + +Fix: + +```sql +revoke execute on function public.exec_sql(text) from anon, authenticated, public; +``` + +### False Positives + +- A `text` parameter used as an identifier through `format('%I', param)` is safe but can still be flagged. Use `quote_ident(param)` instead. +- A function that checks the string against a strict allow-list before running it. Confirm the allow-list is complete, then suppress the finding. +- Only PL/pgSQL bodies are inspected. Functions in other languages, or that build SQL in a helper function, are not detected. diff --git a/lints/0031_anon_dynamic_sql_function_executable.sql b/lints/0031_anon_dynamic_sql_function_executable.sql new file mode 100644 index 00000000..0b4f007d --- /dev/null +++ b/lints/0031_anon_dynamic_sql_function_executable.sql @@ -0,0 +1,161 @@ +create view lint."0031_anon_dynamic_sql_function_executable" as + +-- Detects PL/pgSQL functions that the `anon` role can EXECUTE and that +-- appear to EXECUTE a SQL string passed in by the caller, e.g. +-- `exec_sql(query text)`. PostgREST exposes them at +-- `/rest/v1/rpc/`, so anyone with the anon key can run arbitrary SQL as the +-- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). +-- +-- A function is flagged when either: +-- 1. a text/varchar parameter is used inside an EXECUTE statement +-- (EXECUTE ... USING bound values and quote_ident/quote_literal/ +-- quote_nullable(param) are ignored), or +-- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- ...) and the body contains an EXECUTE anywhere. +-- +-- Level is ERROR for SECURITY DEFINER functions matching (1), WARN +-- otherwise. Unlike 0028/0029 this also covers SECURITY INVOKER. +-- +-- See lint 0032 for the equivalent check against `authenticated`. +with candidates as ( + select + n.nspname as schema_name, + p.proname as function_name, + pg_catalog.pg_get_function_identity_arguments(p.oid) as function_args, + l.lanname as function_language, + p.prosecdef as is_security_definer, + pg_catalog.pg_get_userbyid(p.proowner) as owner_name, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + p.proargnames, + p.proargmodes, + coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types + from + pg_catalog.pg_proc p + join pg_catalog.pg_namespace n + on p.pronamespace = n.oid + join pg_catalog.pg_language l + on p.prolang = l.oid + where + l.lanname = 'plpgsql' + and pg_catalog.has_function_privilege('anon', p.oid, 'EXECUTE') + and n.nspname = any(array(select trim(unnest(string_to_array(coalesce(current_setting('pgrst.db_schemas', 't'), 'public'), ','))))) + and n.nspname not in ( + '_timescaledb_cache', '_timescaledb_catalog', '_timescaledb_config', '_timescaledb_internal', 'auth', 'cron', 'extensions', 'graphql', 'graphql_public', 'information_schema', 'net', 'pgmq', 'pgroonga', 'pgsodium', 'pgsodium_masks', 'pgtle', 'pgbouncer', 'pg_catalog', 'realtime', 'repack', 'storage', 'supabase_functions', 'supabase_migrations', 'tiger', 'topology', 'vault' + ) + -- exclude functions that belong to an extension + and not exists ( + select 1 + from pg_catalog.pg_depend d + where d.objid = p.oid + and d.classid = 'pg_catalog.pg_proc'::pg_catalog.regclass + and d.deptype = 'e' + ) + and lower(p.prosrc) like '%execute%' +), +text_params as ( + select + c.*, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + from + candidates c + cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + where + coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') + and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) +), +escaped as ( + select + tp.*, + regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + from + text_params tp +), +cleaned as ( + select + e.*, + -- ignore `USING` bound values and quote_*(param) wrappers + regexp_replace( + regexp_replace(e.body, '\musing\s+[^(;\s][^;]*', '', 'g'), + 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + '', + 'g' + ) as exec_body + from + escaped e +), +matches as ( + select + cl.schema_name, + cl.function_name, + cl.function_args, + cl.function_language, + cl.is_security_definer, + cl.owner_name, + cl.arg_name, + cl.exec_body ~ ('\mexecute\M[^;]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') + and cl.exec_body ~ '\mexecute\M' as renamed_use + from + cleaned cl +), +flagged as ( + select + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name, + bool_or(direct_use) as has_direct_use, + array_agg(arg_name order by arg_name) as sql_parameters + from + matches + where + direct_use or renamed_use + group by + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name +) +select + 'anon_dynamic_sql_function_executable' as name, + 'Public Can Execute Dynamic SQL Function' as title, + case when is_security_definer and has_direct_use then 'ERROR' else 'WARN' end as level, + 'EXTERNAL' as facing, + array['SECURITY'] as categories, + 'Detects functions callable without signing in that appear to run a SQL string passed in by the caller. Revoke `EXECUTE` or replace the function with fixed operations.' as description, + format( + 'Function `%s.%s(%s)` is `SECURITY %s`, can be executed by the `anon` role via `/rest/v1/rpc/%s`, and appears to run SQL passed in `%s`. Revoke `EXECUTE` if that is not intentional.', + schema_name, + function_name, + function_args, + case when is_security_definer then 'DEFINER' else 'INVOKER' end, + function_name, + array_to_string(sql_parameters, ', ') + ) as detail, + 'https://supabase.com/docs/guides/database/database-linter?lint=0031_anon_dynamic_sql_function_executable' as remediation, + jsonb_build_object( + 'schema', schema_name, + 'name', function_name, + 'type', 'function', + 'arguments', function_args, + 'language', function_language, + 'security_definer', is_security_definer, + 'owner', owner_name, + 'sql_parameters', sql_parameters + ) as metadata, + format( + 'anon_dynamic_sql_function_executable_%s_%s_%s', + schema_name, + function_name, + function_args + ) as cache_key +from + flagged +order by + schema_name, + function_name, + function_args; diff --git a/lints/0032_authenticated_dynamic_sql_function_executable.sql b/lints/0032_authenticated_dynamic_sql_function_executable.sql new file mode 100644 index 00000000..53867511 --- /dev/null +++ b/lints/0032_authenticated_dynamic_sql_function_executable.sql @@ -0,0 +1,161 @@ +create view lint."0032_authenticated_dynamic_sql_function_executable" as + +-- Detects PL/pgSQL functions that the `authenticated` role can EXECUTE and that +-- appear to EXECUTE a SQL string passed in by the caller, e.g. +-- `exec_sql(query text)`. PostgREST exposes them at +-- `/rest/v1/rpc/`, so any signed-in user can run arbitrary SQL as the +-- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). +-- +-- A function is flagged when either: +-- 1. a text/varchar parameter is used inside an EXECUTE statement +-- (EXECUTE ... USING bound values and quote_ident/quote_literal/ +-- quote_nullable(param) are ignored), or +-- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- ...) and the body contains an EXECUTE anywhere. +-- +-- Level is ERROR for SECURITY DEFINER functions matching (1), WARN +-- otherwise. Unlike 0028/0029 this also covers SECURITY INVOKER. +-- +-- See lint 0031 for the equivalent check against `anon`. +with candidates as ( + select + n.nspname as schema_name, + p.proname as function_name, + pg_catalog.pg_get_function_identity_arguments(p.oid) as function_args, + l.lanname as function_language, + p.prosecdef as is_security_definer, + pg_catalog.pg_get_userbyid(p.proowner) as owner_name, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + p.proargnames, + p.proargmodes, + coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types + from + pg_catalog.pg_proc p + join pg_catalog.pg_namespace n + on p.pronamespace = n.oid + join pg_catalog.pg_language l + on p.prolang = l.oid + where + l.lanname = 'plpgsql' + and pg_catalog.has_function_privilege('authenticated', p.oid, 'EXECUTE') + and n.nspname = any(array(select trim(unnest(string_to_array(coalesce(current_setting('pgrst.db_schemas', 't'), 'public'), ','))))) + and n.nspname not in ( + '_timescaledb_cache', '_timescaledb_catalog', '_timescaledb_config', '_timescaledb_internal', 'auth', 'cron', 'extensions', 'graphql', 'graphql_public', 'information_schema', 'net', 'pgmq', 'pgroonga', 'pgsodium', 'pgsodium_masks', 'pgtle', 'pgbouncer', 'pg_catalog', 'realtime', 'repack', 'storage', 'supabase_functions', 'supabase_migrations', 'tiger', 'topology', 'vault' + ) + -- exclude functions that belong to an extension + and not exists ( + select 1 + from pg_catalog.pg_depend d + where d.objid = p.oid + and d.classid = 'pg_catalog.pg_proc'::pg_catalog.regclass + and d.deptype = 'e' + ) + and lower(p.prosrc) like '%execute%' +), +text_params as ( + select + c.*, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + from + candidates c + cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + where + coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') + and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) +), +escaped as ( + select + tp.*, + regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + from + text_params tp +), +cleaned as ( + select + e.*, + -- ignore `USING` bound values and quote_*(param) wrappers + regexp_replace( + regexp_replace(e.body, '\musing\s+[^(;\s][^;]*', '', 'g'), + 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + '', + 'g' + ) as exec_body + from + escaped e +), +matches as ( + select + cl.schema_name, + cl.function_name, + cl.function_args, + cl.function_language, + cl.is_security_definer, + cl.owner_name, + cl.arg_name, + cl.exec_body ~ ('\mexecute\M[^;]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') + and cl.exec_body ~ '\mexecute\M' as renamed_use + from + cleaned cl +), +flagged as ( + select + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name, + bool_or(direct_use) as has_direct_use, + array_agg(arg_name order by arg_name) as sql_parameters + from + matches + where + direct_use or renamed_use + group by + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name +) +select + 'authenticated_dynamic_sql_function_executable' as name, + 'Signed-In Users Can Execute Dynamic SQL Function' as title, + case when is_security_definer and has_direct_use then 'ERROR' else 'WARN' end as level, + 'EXTERNAL' as facing, + array['SECURITY'] as categories, + 'Detects functions callable by signed-in users that appear to run a SQL string passed in by the caller. Revoke `EXECUTE` or replace the function with fixed operations.' as description, + format( + 'Function `%s.%s(%s)` is `SECURITY %s`, can be executed by the `authenticated` role via `/rest/v1/rpc/%s`, and appears to run SQL passed in `%s`. Revoke `EXECUTE` if that is not intentional.', + schema_name, + function_name, + function_args, + case when is_security_definer then 'DEFINER' else 'INVOKER' end, + function_name, + array_to_string(sql_parameters, ', ') + ) as detail, + 'https://supabase.com/docs/guides/database/database-linter?lint=0032_authenticated_dynamic_sql_function_executable' as remediation, + jsonb_build_object( + 'schema', schema_name, + 'name', function_name, + 'type', 'function', + 'arguments', function_args, + 'language', function_language, + 'security_definer', is_security_definer, + 'owner', owner_name, + 'sql_parameters', sql_parameters + ) as metadata, + format( + 'authenticated_dynamic_sql_function_executable_%s_%s_%s', + schema_name, + function_name, + function_args + ) as cache_key +from + flagged +order by + schema_name, + function_name, + function_args; diff --git a/mkdocs.yaml b/mkdocs.yaml index 02864ac9..bf45eb55 100644 --- a/mkdocs.yaml +++ b/mkdocs.yaml @@ -38,6 +38,8 @@ nav: - Public Can Execute SECURITY DEFINER Function: '0028_anon_security_definer_function_executable.md' - Signed-In Users Can Execute SECURITY DEFINER Function: '0029_authenticated_security_definer_function_executable.md' - Autovacuum Disabled: '0030_autovacuum_disabled.md' + - Public Can Execute Dynamic SQL Function: '0031_anon_dynamic_sql_function_executable.md' + - Signed-In Users Can Execute Dynamic SQL Function: '0032_authenticated_dynamic_sql_function_executable.md' theme: name: 'material' diff --git a/splinter.sql b/splinter.sql index 46bb331e..504a5bfa 100644 --- a/splinter.sql +++ b/splinter.sql @@ -1874,4 +1874,326 @@ where ) order by nsp.nspname, - cls.relname) \ No newline at end of file + cls.relname) +union all +( +-- Detects PL/pgSQL functions that the `anon` role can EXECUTE and that +-- appear to EXECUTE a SQL string passed in by the caller, e.g. +-- `exec_sql(query text)`. PostgREST exposes them at +-- `/rest/v1/rpc/`, so anyone with the anon key can run arbitrary SQL as the +-- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). +-- +-- A function is flagged when either: +-- 1. a text/varchar parameter is used inside an EXECUTE statement +-- (EXECUTE ... USING bound values and quote_ident/quote_literal/ +-- quote_nullable(param) are ignored), or +-- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- ...) and the body contains an EXECUTE anywhere. +-- +-- Level is ERROR for SECURITY DEFINER functions matching (1), WARN +-- otherwise. Unlike 0028/0029 this also covers SECURITY INVOKER. +-- +-- See lint 0032 for the equivalent check against `authenticated`. +with candidates as ( + select + n.nspname as schema_name, + p.proname as function_name, + pg_catalog.pg_get_function_identity_arguments(p.oid) as function_args, + l.lanname as function_language, + p.prosecdef as is_security_definer, + pg_catalog.pg_get_userbyid(p.proowner) as owner_name, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + p.proargnames, + p.proargmodes, + coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types + from + pg_catalog.pg_proc p + join pg_catalog.pg_namespace n + on p.pronamespace = n.oid + join pg_catalog.pg_language l + on p.prolang = l.oid + where + l.lanname = 'plpgsql' + and pg_catalog.has_function_privilege('anon', p.oid, 'EXECUTE') + and n.nspname = any(array(select trim(unnest(string_to_array(coalesce(current_setting('pgrst.db_schemas', 't'), 'public'), ','))))) + and n.nspname not in ( + '_timescaledb_cache', '_timescaledb_catalog', '_timescaledb_config', '_timescaledb_internal', 'auth', 'cron', 'extensions', 'graphql', 'graphql_public', 'information_schema', 'net', 'pgmq', 'pgroonga', 'pgsodium', 'pgsodium_masks', 'pgtle', 'pgbouncer', 'pg_catalog', 'realtime', 'repack', 'storage', 'supabase_functions', 'supabase_migrations', 'tiger', 'topology', 'vault' + ) + -- exclude functions that belong to an extension + and not exists ( + select 1 + from pg_catalog.pg_depend d + where d.objid = p.oid + and d.classid = 'pg_catalog.pg_proc'::pg_catalog.regclass + and d.deptype = 'e' + ) + and lower(p.prosrc) like '%execute%' +), +text_params as ( + select + c.*, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + from + candidates c + cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + where + coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') + and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) +), +escaped as ( + select + tp.*, + regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + from + text_params tp +), +cleaned as ( + select + e.*, + -- ignore `USING` bound values and quote_*(param) wrappers + regexp_replace( + regexp_replace(e.body, '\musing\s+[^(\s][^]*', '', 'g'), + 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + '', + 'g' + ) as exec_body + from + escaped e +), +matches as ( + select + cl.schema_name, + cl.function_name, + cl.function_args, + cl.function_language, + cl.is_security_definer, + cl.owner_name, + cl.arg_name, + cl.exec_body ~ ('\mexecute\M[^]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') + and cl.exec_body ~ '\mexecute\M' as renamed_use + from + cleaned cl +), +flagged as ( + select + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name, + bool_or(direct_use) as has_direct_use, + array_agg(arg_name order by arg_name) as sql_parameters + from + matches + where + direct_use or renamed_use + group by + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name +) +select + 'anon_dynamic_sql_function_executable' as name, + 'Public Can Execute Dynamic SQL Function' as title, + case when is_security_definer and has_direct_use then 'ERROR' else 'WARN' end as level, + 'EXTERNAL' as facing, + array['SECURITY'] as categories, + 'Detects functions callable without signing in that appear to run a SQL string passed in by the caller. Revoke `EXECUTE` or replace the function with fixed operations.' as description, + format( + 'Function `%s.%s(%s)` is `SECURITY %s`, can be executed by the `anon` role via `/rest/v1/rpc/%s`, and appears to run SQL passed in `%s`. Revoke `EXECUTE` if that is not intentional.', + schema_name, + function_name, + function_args, + case when is_security_definer then 'DEFINER' else 'INVOKER' end, + function_name, + array_to_string(sql_parameters, ', ') + ) as detail, + 'https://supabase.com/docs/guides/database/database-linter?lint=0031_anon_dynamic_sql_function_executable' as remediation, + jsonb_build_object( + 'schema', schema_name, + 'name', function_name, + 'type', 'function', + 'arguments', function_args, + 'language', function_language, + 'security_definer', is_security_definer, + 'owner', owner_name, + 'sql_parameters', sql_parameters + ) as metadata, + format( + 'anon_dynamic_sql_function_executable_%s_%s_%s', + schema_name, + function_name, + function_args + ) as cache_key +from + flagged +order by + schema_name, + function_name, + function_args) +union all +( +-- Detects PL/pgSQL functions that the `authenticated` role can EXECUTE and that +-- appear to EXECUTE a SQL string passed in by the caller, e.g. +-- `exec_sql(query text)`. PostgREST exposes them at +-- `/rest/v1/rpc/`, so any signed-in user can run arbitrary SQL as the +-- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). +-- +-- A function is flagged when either: +-- 1. a text/varchar parameter is used inside an EXECUTE statement +-- (EXECUTE ... USING bound values and quote_ident/quote_literal/ +-- quote_nullable(param) are ignored), or +-- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- ...) and the body contains an EXECUTE anywhere. +-- +-- Level is ERROR for SECURITY DEFINER functions matching (1), WARN +-- otherwise. Unlike 0028/0029 this also covers SECURITY INVOKER. +-- +-- See lint 0031 for the equivalent check against `anon`. +with candidates as ( + select + n.nspname as schema_name, + p.proname as function_name, + pg_catalog.pg_get_function_identity_arguments(p.oid) as function_args, + l.lanname as function_language, + p.prosecdef as is_security_definer, + pg_catalog.pg_get_userbyid(p.proowner) as owner_name, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + p.proargnames, + p.proargmodes, + coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types + from + pg_catalog.pg_proc p + join pg_catalog.pg_namespace n + on p.pronamespace = n.oid + join pg_catalog.pg_language l + on p.prolang = l.oid + where + l.lanname = 'plpgsql' + and pg_catalog.has_function_privilege('authenticated', p.oid, 'EXECUTE') + and n.nspname = any(array(select trim(unnest(string_to_array(coalesce(current_setting('pgrst.db_schemas', 't'), 'public'), ','))))) + and n.nspname not in ( + '_timescaledb_cache', '_timescaledb_catalog', '_timescaledb_config', '_timescaledb_internal', 'auth', 'cron', 'extensions', 'graphql', 'graphql_public', 'information_schema', 'net', 'pgmq', 'pgroonga', 'pgsodium', 'pgsodium_masks', 'pgtle', 'pgbouncer', 'pg_catalog', 'realtime', 'repack', 'storage', 'supabase_functions', 'supabase_migrations', 'tiger', 'topology', 'vault' + ) + -- exclude functions that belong to an extension + and not exists ( + select 1 + from pg_catalog.pg_depend d + where d.objid = p.oid + and d.classid = 'pg_catalog.pg_proc'::pg_catalog.regclass + and d.deptype = 'e' + ) + and lower(p.prosrc) like '%execute%' +), +text_params as ( + select + c.*, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + from + candidates c + cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + where + coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') + and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) +), +escaped as ( + select + tp.*, + regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + from + text_params tp +), +cleaned as ( + select + e.*, + -- ignore `USING` bound values and quote_*(param) wrappers + regexp_replace( + regexp_replace(e.body, '\musing\s+[^(\s][^]*', '', 'g'), + 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + '', + 'g' + ) as exec_body + from + escaped e +), +matches as ( + select + cl.schema_name, + cl.function_name, + cl.function_args, + cl.function_language, + cl.is_security_definer, + cl.owner_name, + cl.arg_name, + cl.exec_body ~ ('\mexecute\M[^]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') + and cl.exec_body ~ '\mexecute\M' as renamed_use + from + cleaned cl +), +flagged as ( + select + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name, + bool_or(direct_use) as has_direct_use, + array_agg(arg_name order by arg_name) as sql_parameters + from + matches + where + direct_use or renamed_use + group by + schema_name, + function_name, + function_args, + function_language, + is_security_definer, + owner_name +) +select + 'authenticated_dynamic_sql_function_executable' as name, + 'Signed-In Users Can Execute Dynamic SQL Function' as title, + case when is_security_definer and has_direct_use then 'ERROR' else 'WARN' end as level, + 'EXTERNAL' as facing, + array['SECURITY'] as categories, + 'Detects functions callable by signed-in users that appear to run a SQL string passed in by the caller. Revoke `EXECUTE` or replace the function with fixed operations.' as description, + format( + 'Function `%s.%s(%s)` is `SECURITY %s`, can be executed by the `authenticated` role via `/rest/v1/rpc/%s`, and appears to run SQL passed in `%s`. Revoke `EXECUTE` if that is not intentional.', + schema_name, + function_name, + function_args, + case when is_security_definer then 'DEFINER' else 'INVOKER' end, + function_name, + array_to_string(sql_parameters, ', ') + ) as detail, + 'https://supabase.com/docs/guides/database/database-linter?lint=0032_authenticated_dynamic_sql_function_executable' as remediation, + jsonb_build_object( + 'schema', schema_name, + 'name', function_name, + 'type', 'function', + 'arguments', function_args, + 'language', function_language, + 'security_definer', is_security_definer, + 'owner', owner_name, + 'sql_parameters', sql_parameters + ) as metadata, + format( + 'authenticated_dynamic_sql_function_executable_%s_%s_%s', + schema_name, + function_name, + function_args + ) as cache_key +from + flagged +order by + schema_name, + function_name, + function_args) \ No newline at end of file diff --git a/test/expected/0031_anon_dynamic_sql_function_executable.out b/test/expected/0031_anon_dynamic_sql_function_executable.out new file mode 100644 index 00000000..c0f5dd8d --- /dev/null +++ b/test/expected/0031_anon_dynamic_sql_function_executable.out @@ -0,0 +1,186 @@ +begin; + set local search_path = ''; + set local pgrst.db_schemas = 'public'; + -- BASELINE: no functions, no rows + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + savepoint a; + ---------------------------------------- + -- NEGATIVE: dynamic SQL, but the text parameter is only passed via + -- EXECUTE ... USING (bound value) and the SQL text is static. + ---------------------------------------- + create function public.find_user(p_name text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from auth.users where email = $1' into r using p_name; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: parameter only used through quote_literal/quote_ident + create function public.quoted(tbl text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from ' || quote_ident(tbl) into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: no EXECUTE at all, even with a SQL-shaped parameter name + create function public.search(query text) returns int + language plpgsql + security definer + as $$ + begin + return length(query); + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: EXECUTE only mentioned in a comment + create function public.commented(query text) returns int + language plpgsql + as $$ + begin + -- we used to execute query here + return length(query); + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint a; + savepoint case_definer; + ---------------------------------------- + -- POSITIVE (ERROR): the exec_sql shape. SECURITY DEFINER, the + -- caller-supplied string is concatenated into an EXECUTE. + ---------------------------------------- + create function public.exec_sql(query text) returns json + language plpgsql + security definer + as $$ + declare r json; + begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; + end $$; + select + name, + level, + detail, + metadata->>'security_definer' as security_definer, + metadata->'sql_parameters' as sql_parameters, + cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | detail | security_definer | sql_parameters | cache_key +--------------------------------------+-------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+------------------+----------------+----------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | Function `public.exec_sql(query text)` is `SECURITY DEFINER`, can be executed by the `anon` role via `/rest/v1/rpc/exec_sql`, and appears to run SQL passed in `query`. Revoke `EXECUTE` if that is not intentional. | true | ["query"] | anon_dynamic_sql_function_executable_public_exec_sql_query text +(1 row) + + ---------------------------------------- + -- RESOLUTION: revoke EXECUTE from anon and PUBLIC + ---------------------------------------- + revoke execute on function public.exec_sql(text) from anon, public; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint case_definer; + savepoint case_invoker; + ---------------------------------------- + -- POSITIVE (WARN): same shape but SECURITY INVOKER, the function runs + -- as anon. Not covered by lint 0028. + ---------------------------------------- + create function public.run_query(sql text) returns void + language plpgsql + security invoker + as $$ + begin + execute sql; + end $$; + select name, level, metadata->>'security_definer' as security_definer, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | security_definer | cache_key +--------------------------------------+-------+------------------+---------------------------------------------------------------- + anon_dynamic_sql_function_executable | WARN | false | anon_dynamic_sql_function_executable_public_run_query_sql text +(1 row) + + rollback to savepoint case_invoker; + savepoint case_renamed; + ---------------------------------------- + -- POSITIVE (WARN): renamed wrapper that copies the SQL parameter into a + -- local variable before EXECUTE, so it is not referenced directly. + ---------------------------------------- + create function public.insert_entries(sql text) returns void + language plpgsql + security definer + as $$ + declare stmt text; + begin + stmt := sql; + execute stmt; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +--------------------------------------+-------+----------------+--------------------------------------------------------------------- + anon_dynamic_sql_function_executable | WARN | ["sql"] | anon_dynamic_sql_function_executable_public_insert_entries_sql text +(1 row) + + rollback to savepoint case_renamed; + savepoint case_unnamed; + ---------------------------------------- + -- POSITIVE (ERROR): unnamed parameter used as `EXECUTE $1` + ---------------------------------------- + create function public.exec_unnamed(text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +--------------------------------------+-------+----------------+--------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | ["$1"] | anon_dynamic_sql_function_executable_public_exec_unnamed_text +(1 row) + + rollback to savepoint case_unnamed; + savepoint case_system_schema; + ---------------------------------------- + -- NEGATIVE: function in a system schema is excluded + ---------------------------------------- + create function auth.exec_sql(query text) returns void + language plpgsql + security definer + as $$ + begin + execute query; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint case_system_schema; +rollback; diff --git a/test/expected/0032_authenticated_dynamic_sql_function_executable.out b/test/expected/0032_authenticated_dynamic_sql_function_executable.out new file mode 100644 index 00000000..0e97f604 --- /dev/null +++ b/test/expected/0032_authenticated_dynamic_sql_function_executable.out @@ -0,0 +1,186 @@ +begin; + set local search_path = ''; + set local pgrst.db_schemas = 'public'; + -- BASELINE: no functions, no rows + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + savepoint a; + ---------------------------------------- + -- NEGATIVE: dynamic SQL, but the text parameter is only passed via + -- EXECUTE ... USING (bound value) and the SQL text is static. + ---------------------------------------- + create function public.find_user(p_name text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from auth.users where email = $1' into r using p_name; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: parameter only used through quote_literal/quote_ident + create function public.quoted(tbl text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from ' || quote_ident(tbl) into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: no EXECUTE at all, even with a SQL-shaped parameter name + create function public.search(query text) returns int + language plpgsql + security definer + as $$ + begin + return length(query); + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: EXECUTE only mentioned in a comment + create function public.commented(query text) returns int + language plpgsql + as $$ + begin + -- we used to execute query here + return length(query); + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint a; + savepoint case_definer; + ---------------------------------------- + -- POSITIVE (ERROR): the exec_sql shape. SECURITY DEFINER, the + -- caller-supplied string is concatenated into an EXECUTE. + ---------------------------------------- + create function public.exec_sql(query text) returns json + language plpgsql + security definer + as $$ + declare r json; + begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; + end $$; + select + name, + level, + detail, + metadata->>'security_definer' as security_definer, + metadata->'sql_parameters' as sql_parameters, + cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | detail | security_definer | sql_parameters | cache_key +-----------------------------------------------+-------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+------------------+----------------+-------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | Function `public.exec_sql(query text)` is `SECURITY DEFINER`, can be executed by the `authenticated` role via `/rest/v1/rpc/exec_sql`, and appears to run SQL passed in `query`. Revoke `EXECUTE` if that is not intentional. | true | ["query"] | authenticated_dynamic_sql_function_executable_public_exec_sql_query text +(1 row) + + ---------------------------------------- + -- RESOLUTION: revoke EXECUTE from authenticated and PUBLIC + ---------------------------------------- + revoke execute on function public.exec_sql(text) from authenticated, public; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint case_definer; + savepoint case_invoker; + ---------------------------------------- + -- POSITIVE (WARN): same shape but SECURITY INVOKER, the function runs + -- as authenticated. Not covered by lint 0029. + ---------------------------------------- + create function public.run_query(sql text) returns void + language plpgsql + security invoker + as $$ + begin + execute sql; + end $$; + select name, level, metadata->>'security_definer' as security_definer, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | security_definer | cache_key +-----------------------------------------------+-------+------------------+------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | WARN | false | authenticated_dynamic_sql_function_executable_public_run_query_sql text +(1 row) + + rollback to savepoint case_invoker; + savepoint case_renamed; + ---------------------------------------- + -- POSITIVE (WARN): renamed wrapper that copies the SQL parameter into a + -- local variable before EXECUTE, so it is not referenced directly. + ---------------------------------------- + create function public.insert_entries(sql text) returns void + language plpgsql + security definer + as $$ + declare stmt text; + begin + stmt := sql; + execute stmt; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +-----------------------------------------------+-------+----------------+------------------------------------------------------------------------------ + authenticated_dynamic_sql_function_executable | WARN | ["sql"] | authenticated_dynamic_sql_function_executable_public_insert_entries_sql text +(1 row) + + rollback to savepoint case_renamed; + savepoint case_unnamed; + ---------------------------------------- + -- POSITIVE (ERROR): unnamed parameter used as `EXECUTE $1` + ---------------------------------------- + create function public.exec_unnamed(text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +-----------------------------------------------+-------+----------------+------------------------------------------------------------------------ + authenticated_dynamic_sql_function_executable | ERROR | ["$1"] | authenticated_dynamic_sql_function_executable_public_exec_unnamed_text +(1 row) + + rollback to savepoint case_unnamed; + savepoint case_system_schema; + ---------------------------------------- + -- NEGATIVE: function in a system schema is excluded + ---------------------------------------- + create function auth.exec_sql(query text) returns void + language plpgsql + security definer + as $$ + begin + execute query; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + rollback to savepoint case_system_schema; +rollback; diff --git a/test/expected/queries_are_unionable.out b/test/expected/queries_are_unionable.out index 058f2dd0..e7738bd7 100644 --- a/test/expected/queries_are_unionable.out +++ b/test/expected/queries_are_unionable.out @@ -56,7 +56,11 @@ begin; union all select * from lint."0029_authenticated_security_definer_function_executable" union all - select * from lint."0030_autovacuum_disabled"; + select * from lint."0030_autovacuum_disabled" + union all + select * from lint."0031_anon_dynamic_sql_function_executable" + union all + select * from lint."0032_authenticated_dynamic_sql_function_executable"; name | title | level | facing | categories | description | detail | remediation | metadata | cache_key ------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- (0 rows) diff --git a/test/sql/0031_anon_dynamic_sql_function_executable.sql b/test/sql/0031_anon_dynamic_sql_function_executable.sql new file mode 100644 index 00000000..94798928 --- /dev/null +++ b/test/sql/0031_anon_dynamic_sql_function_executable.sql @@ -0,0 +1,166 @@ +begin; + set local search_path = ''; + set local pgrst.db_schemas = 'public'; + + -- BASELINE: no functions, no rows + select * from lint."0031_anon_dynamic_sql_function_executable"; + + savepoint a; + + ---------------------------------------- + -- NEGATIVE: dynamic SQL, but the text parameter is only passed via + -- EXECUTE ... USING (bound value) and the SQL text is static. + ---------------------------------------- + create function public.find_user(p_name text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from auth.users where email = $1' into r using p_name; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- NEGATIVE: parameter only used through quote_literal/quote_ident + create function public.quoted(tbl text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from ' || quote_ident(tbl) into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- NEGATIVE: no EXECUTE at all, even with a SQL-shaped parameter name + create function public.search(query text) returns int + language plpgsql + security definer + as $$ + begin + return length(query); + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- NEGATIVE: EXECUTE only mentioned in a comment + create function public.commented(query text) returns int + language plpgsql + as $$ + begin + -- we used to execute query here + return length(query); + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint a; + + savepoint case_definer; + + ---------------------------------------- + -- POSITIVE (ERROR): the exec_sql shape. SECURITY DEFINER, the + -- caller-supplied string is concatenated into an EXECUTE. + ---------------------------------------- + create function public.exec_sql(query text) returns json + language plpgsql + security definer + as $$ + declare r json; + begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; + end $$; + + select + name, + level, + detail, + metadata->>'security_definer' as security_definer, + metadata->'sql_parameters' as sql_parameters, + cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + ---------------------------------------- + -- RESOLUTION: revoke EXECUTE from anon and PUBLIC + ---------------------------------------- + revoke execute on function public.exec_sql(text) from anon, public; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_definer; + + savepoint case_invoker; + + ---------------------------------------- + -- POSITIVE (WARN): same shape but SECURITY INVOKER, the function runs + -- as anon. Not covered by lint 0028. + ---------------------------------------- + create function public.run_query(sql text) returns void + language plpgsql + security invoker + as $$ + begin + execute sql; + end $$; + + select name, level, metadata->>'security_definer' as security_definer, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_invoker; + + savepoint case_renamed; + + ---------------------------------------- + -- POSITIVE (WARN): renamed wrapper that copies the SQL parameter into a + -- local variable before EXECUTE, so it is not referenced directly. + ---------------------------------------- + create function public.insert_entries(sql text) returns void + language plpgsql + security definer + as $$ + declare stmt text; + begin + stmt := sql; + execute stmt; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_renamed; + + savepoint case_unnamed; + + ---------------------------------------- + -- POSITIVE (ERROR): unnamed parameter used as `EXECUTE $1` + ---------------------------------------- + create function public.exec_unnamed(text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_unnamed; + + savepoint case_system_schema; + + ---------------------------------------- + -- NEGATIVE: function in a system schema is excluded + ---------------------------------------- + create function auth.exec_sql(query text) returns void + language plpgsql + security definer + as $$ + begin + execute query; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_system_schema; + +rollback; diff --git a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql new file mode 100644 index 00000000..e4b3410a --- /dev/null +++ b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql @@ -0,0 +1,166 @@ +begin; + set local search_path = ''; + set local pgrst.db_schemas = 'public'; + + -- BASELINE: no functions, no rows + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + savepoint a; + + ---------------------------------------- + -- NEGATIVE: dynamic SQL, but the text parameter is only passed via + -- EXECUTE ... USING (bound value) and the SQL text is static. + ---------------------------------------- + create function public.find_user(p_name text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from auth.users where email = $1' into r using p_name; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- NEGATIVE: parameter only used through quote_literal/quote_ident + create function public.quoted(tbl text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute 'select count(*)::int from ' || quote_ident(tbl) into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- NEGATIVE: no EXECUTE at all, even with a SQL-shaped parameter name + create function public.search(query text) returns int + language plpgsql + security definer + as $$ + begin + return length(query); + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- NEGATIVE: EXECUTE only mentioned in a comment + create function public.commented(query text) returns int + language plpgsql + as $$ + begin + -- we used to execute query here + return length(query); + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint a; + + savepoint case_definer; + + ---------------------------------------- + -- POSITIVE (ERROR): the exec_sql shape. SECURITY DEFINER, the + -- caller-supplied string is concatenated into an EXECUTE. + ---------------------------------------- + create function public.exec_sql(query text) returns json + language plpgsql + security definer + as $$ + declare r json; + begin + execute 'select json_agg(t) from (' || query || ') t' into r; + return r; + end $$; + + select + name, + level, + detail, + metadata->>'security_definer' as security_definer, + metadata->'sql_parameters' as sql_parameters, + cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + ---------------------------------------- + -- RESOLUTION: revoke EXECUTE from authenticated and PUBLIC + ---------------------------------------- + revoke execute on function public.exec_sql(text) from authenticated, public; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_definer; + + savepoint case_invoker; + + ---------------------------------------- + -- POSITIVE (WARN): same shape but SECURITY INVOKER, the function runs + -- as authenticated. Not covered by lint 0029. + ---------------------------------------- + create function public.run_query(sql text) returns void + language plpgsql + security invoker + as $$ + begin + execute sql; + end $$; + + select name, level, metadata->>'security_definer' as security_definer, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_invoker; + + savepoint case_renamed; + + ---------------------------------------- + -- POSITIVE (WARN): renamed wrapper that copies the SQL parameter into a + -- local variable before EXECUTE, so it is not referenced directly. + ---------------------------------------- + create function public.insert_entries(sql text) returns void + language plpgsql + security definer + as $$ + declare stmt text; + begin + stmt := sql; + execute stmt; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_renamed; + + savepoint case_unnamed; + + ---------------------------------------- + -- POSITIVE (ERROR): unnamed parameter used as `EXECUTE $1` + ---------------------------------------- + create function public.exec_unnamed(text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_unnamed; + + savepoint case_system_schema; + + ---------------------------------------- + -- NEGATIVE: function in a system schema is excluded + ---------------------------------------- + create function auth.exec_sql(query text) returns void + language plpgsql + security definer + as $$ + begin + execute query; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_system_schema; + +rollback; diff --git a/test/sql/queries_are_unionable.sql b/test/sql/queries_are_unionable.sql index 3c5c39d3..6169d029 100644 --- a/test/sql/queries_are_unionable.sql +++ b/test/sql/queries_are_unionable.sql @@ -58,6 +58,10 @@ begin; union all select * from lint."0029_authenticated_security_definer_function_executable" union all - select * from lint."0030_autovacuum_disabled"; + select * from lint."0030_autovacuum_disabled" + union all + select * from lint."0031_anon_dynamic_sql_function_executable" + union all + select * from lint."0032_authenticated_dynamic_sql_function_executable"; rollback; From 6a8f1d26daa19a2d4de8472012bc8ae899174f5b Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 14:11:47 +1300 Subject: [PATCH 2/6] fix: regex issue --- lints/0031_anon_dynamic_sql_function_executable.sql | 4 ++-- ...0032_authenticated_dynamic_sql_function_executable.sql | 4 ++-- splinter.sql | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lints/0031_anon_dynamic_sql_function_executable.sql b/lints/0031_anon_dynamic_sql_function_executable.sql index 0b4f007d..aafd2e72 100644 --- a/lints/0031_anon_dynamic_sql_function_executable.sql +++ b/lints/0031_anon_dynamic_sql_function_executable.sql @@ -75,7 +75,7 @@ cleaned as ( e.*, -- ignore `USING` bound values and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(;\s][^;]*', '', 'g'), + regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' @@ -92,7 +92,7 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^;]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from diff --git a/lints/0032_authenticated_dynamic_sql_function_executable.sql b/lints/0032_authenticated_dynamic_sql_function_executable.sql index 53867511..2d4105f2 100644 --- a/lints/0032_authenticated_dynamic_sql_function_executable.sql +++ b/lints/0032_authenticated_dynamic_sql_function_executable.sql @@ -75,7 +75,7 @@ cleaned as ( e.*, -- ignore `USING` bound values and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(;\s][^;]*', '', 'g'), + regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' @@ -92,7 +92,7 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^;]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from diff --git a/splinter.sql b/splinter.sql index 504a5bfa..90699cbc 100644 --- a/splinter.sql +++ b/splinter.sql @@ -1952,7 +1952,7 @@ cleaned as ( e.*, -- ignore `USING` bound values and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\s][^]*', '', 'g'), + regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' @@ -1969,7 +1969,7 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from @@ -2113,7 +2113,7 @@ cleaned as ( e.*, -- ignore `USING` bound values and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\s][^]*', '', 'g'), + regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' @@ -2130,7 +2130,7 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from From e9a1a141664b091b19f9422e9d9f478f291dbe3d Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 14:13:23 +1300 Subject: [PATCH 3/6] fix: check arrays --- ...31_anon_dynamic_sql_function_executable.md | 4 ++-- ...ticated_dynamic_sql_function_executable.md | 4 ++-- ...1_anon_dynamic_sql_function_executable.sql | 11 +++++++--- ...icated_dynamic_sql_function_executable.sql | 11 +++++++--- splinter.sql | 22 ++++++++++++++----- ...1_anon_dynamic_sql_function_executable.out | 19 ++++++++++++++++ ...icated_dynamic_sql_function_executable.out | 19 ++++++++++++++++ ...1_anon_dynamic_sql_function_executable.sql | 18 +++++++++++++++ ...icated_dynamic_sql_function_executable.sql | 18 +++++++++++++++ 9 files changed, 110 insertions(+), 16 deletions(-) diff --git a/docs/0031_anon_dynamic_sql_function_executable.md b/docs/0031_anon_dynamic_sql_function_executable.md index 6d25b9db..e0054a7e 100644 --- a/docs/0031_anon_dynamic_sql_function_executable.md +++ b/docs/0031_anon_dynamic_sql_function_executable.md @@ -14,8 +14,8 @@ Helpers like `exec_sql(query text)` or `run_query(sql text)` let a client send S This lint reads the function body, and only for PL/pgSQL functions in exposed schemas. A function is flagged when: -1. a `text` or `varchar` parameter is used inside an `EXECUTE` statement, or -2. a `text` or `varchar` parameter is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. +1. a `text` or `varchar` parameter (or an array of either, including `VARIADIC`) is used inside an `EXECUTE` statement, or +2. a `text` or `varchar` parameter (or an array of either, including `VARIADIC`) is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. Parameters bound with `EXECUTE ... USING` and parameters wrapped in `quote_ident()`, `quote_literal()` or `quote_nullable()` are not counted. diff --git a/docs/0032_authenticated_dynamic_sql_function_executable.md b/docs/0032_authenticated_dynamic_sql_function_executable.md index 583b9600..a87ab534 100644 --- a/docs/0032_authenticated_dynamic_sql_function_executable.md +++ b/docs/0032_authenticated_dynamic_sql_function_executable.md @@ -14,8 +14,8 @@ Helpers like `exec_sql(query text)` or `run_query(sql text)` let a client send S This lint reads the function body, and only for PL/pgSQL functions in exposed schemas. A function is flagged when: -1. a `text` or `varchar` parameter is used inside an `EXECUTE` statement, or -2. a `text` or `varchar` parameter is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. +1. a `text` or `varchar` parameter (or an array of either, including `VARIADIC`) is used inside an `EXECUTE` statement, or +2. a `text` or `varchar` parameter (or an array of either, including `VARIADIC`) is named like SQL (`sql`, `query`, `stmt`, `statement`, `command`, `cmd`, ...) and the body contains an `EXECUTE` anywhere. Parameters bound with `EXECUTE ... USING` and parameters wrapped in `quote_ident()`, `quote_literal()` or `quote_nullable()` are not counted. diff --git a/lints/0031_anon_dynamic_sql_function_executable.sql b/lints/0031_anon_dynamic_sql_function_executable.sql index aafd2e72..b5fd7203 100644 --- a/lints/0031_anon_dynamic_sql_function_executable.sql +++ b/lints/0031_anon_dynamic_sql_function_executable.sql @@ -7,10 +7,10 @@ create view lint."0031_anon_dynamic_sql_function_executable" as -- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter is used inside an EXECUTE statement +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement -- (EXECUTE ... USING bound values and quote_ident/quote_literal/ -- quote_nullable(param) are ignored), or --- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- -- Level is ERROR for SECURITY DEFINER functions matching (1), WARN @@ -61,7 +61,12 @@ text_params as ( cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') - and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) + and a.typ in ( + 'pg_catalog.text'::pg_catalog.regtype, + 'pg_catalog.varchar'::pg_catalog.regtype, + 'pg_catalog.text[]'::pg_catalog.regtype, + 'pg_catalog.varchar[]'::pg_catalog.regtype + ) ), escaped as ( select diff --git a/lints/0032_authenticated_dynamic_sql_function_executable.sql b/lints/0032_authenticated_dynamic_sql_function_executable.sql index 2d4105f2..886be63a 100644 --- a/lints/0032_authenticated_dynamic_sql_function_executable.sql +++ b/lints/0032_authenticated_dynamic_sql_function_executable.sql @@ -7,10 +7,10 @@ create view lint."0032_authenticated_dynamic_sql_function_executable" as -- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter is used inside an EXECUTE statement +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement -- (EXECUTE ... USING bound values and quote_ident/quote_literal/ -- quote_nullable(param) are ignored), or --- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- -- Level is ERROR for SECURITY DEFINER functions matching (1), WARN @@ -61,7 +61,12 @@ text_params as ( cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') - and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) + and a.typ in ( + 'pg_catalog.text'::pg_catalog.regtype, + 'pg_catalog.varchar'::pg_catalog.regtype, + 'pg_catalog.text[]'::pg_catalog.regtype, + 'pg_catalog.varchar[]'::pg_catalog.regtype + ) ), escaped as ( select diff --git a/splinter.sql b/splinter.sql index 90699cbc..1f7c67a6 100644 --- a/splinter.sql +++ b/splinter.sql @@ -1884,10 +1884,10 @@ union all -- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter is used inside an EXECUTE statement +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement -- (EXECUTE ... USING bound values and quote_ident/quote_literal/ -- quote_nullable(param) are ignored), or --- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- -- Level is ERROR for SECURITY DEFINER functions matching (1), WARN @@ -1938,7 +1938,12 @@ text_params as ( cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') - and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) + and a.typ in ( + 'pg_catalog.text'::pg_catalog.regtype, + 'pg_catalog.varchar'::pg_catalog.regtype, + 'pg_catalog.text[]'::pg_catalog.regtype, + 'pg_catalog.varchar[]'::pg_catalog.regtype + ) ), escaped as ( select @@ -2045,10 +2050,10 @@ union all -- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter is used inside an EXECUTE statement +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement -- (EXECUTE ... USING bound values and quote_ident/quote_literal/ -- quote_nullable(param) are ignored), or --- 2. a text/varchar parameter has a SQL-like name (sql, query, stmt, +-- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- -- Level is ERROR for SECURITY DEFINER functions matching (1), WARN @@ -2099,7 +2104,12 @@ text_params as ( cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') - and a.typ in ('pg_catalog.text'::pg_catalog.regtype, 'pg_catalog.varchar'::pg_catalog.regtype) + and a.typ in ( + 'pg_catalog.text'::pg_catalog.regtype, + 'pg_catalog.varchar'::pg_catalog.regtype, + 'pg_catalog.text[]'::pg_catalog.regtype, + 'pg_catalog.varchar[]'::pg_catalog.regtype + ) ), escaped as ( select diff --git a/test/expected/0031_anon_dynamic_sql_function_executable.out b/test/expected/0031_anon_dynamic_sql_function_executable.out index c0f5dd8d..35a0125b 100644 --- a/test/expected/0031_anon_dynamic_sql_function_executable.out +++ b/test/expected/0031_anon_dynamic_sql_function_executable.out @@ -166,6 +166,25 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_variadic; + ---------------------------------------- + -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE + ---------------------------------------- + create function public.exec_many(variadic queries text[]) returns void + language plpgsql + security definer + as $$ + begin + execute queries[1]; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +--------------------------------------+-------+----------------+------------------------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | ["queries"] | anon_dynamic_sql_function_executable_public_exec_many_VARIADIC queries text[] +(1 row) + + rollback to savepoint case_variadic; savepoint case_system_schema; ---------------------------------------- -- NEGATIVE: function in a system schema is excluded diff --git a/test/expected/0032_authenticated_dynamic_sql_function_executable.out b/test/expected/0032_authenticated_dynamic_sql_function_executable.out index 0e97f604..9c5d144d 100644 --- a/test/expected/0032_authenticated_dynamic_sql_function_executable.out +++ b/test/expected/0032_authenticated_dynamic_sql_function_executable.out @@ -166,6 +166,25 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_variadic; + ---------------------------------------- + -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE + ---------------------------------------- + create function public.exec_many(variadic queries text[]) returns void + language plpgsql + security definer + as $$ + begin + execute queries[1]; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +-----------------------------------------------+-------+----------------+---------------------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | ["queries"] | authenticated_dynamic_sql_function_executable_public_exec_many_VARIADIC queries text[] +(1 row) + + rollback to savepoint case_variadic; savepoint case_system_schema; ---------------------------------------- -- NEGATIVE: function in a system schema is excluded diff --git a/test/sql/0031_anon_dynamic_sql_function_executable.sql b/test/sql/0031_anon_dynamic_sql_function_executable.sql index 94798928..33895bcd 100644 --- a/test/sql/0031_anon_dynamic_sql_function_executable.sql +++ b/test/sql/0031_anon_dynamic_sql_function_executable.sql @@ -147,6 +147,24 @@ begin; rollback to savepoint case_unnamed; + savepoint case_variadic; + + ---------------------------------------- + -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE + ---------------------------------------- + create function public.exec_many(variadic queries text[]) returns void + language plpgsql + security definer + as $$ + begin + execute queries[1]; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_variadic; + savepoint case_system_schema; ---------------------------------------- diff --git a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql index e4b3410a..7eaa7d48 100644 --- a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql +++ b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql @@ -147,6 +147,24 @@ begin; rollback to savepoint case_unnamed; + savepoint case_variadic; + + ---------------------------------------- + -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE + ---------------------------------------- + create function public.exec_many(variadic queries text[]) returns void + language plpgsql + security definer + as $$ + begin + execute queries[1]; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_variadic; + savepoint case_system_schema; ---------------------------------------- From 868ad6408b6961b31fe862f0ad77514c7427ff59 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 15:00:30 +1300 Subject: [PATCH 4/6] handle using string literals --- ...1_anon_dynamic_sql_function_executable.sql | 19 ++++++++-- ...icated_dynamic_sql_function_executable.sql | 19 ++++++++-- splinter.sql | 38 ++++++++++++++++--- ...1_anon_dynamic_sql_function_executable.out | 30 +++++++++++++++ ...icated_dynamic_sql_function_executable.out | 30 +++++++++++++++ ...1_anon_dynamic_sql_function_executable.sql | 28 ++++++++++++++ ...icated_dynamic_sql_function_executable.sql | 28 ++++++++++++++ 7 files changed, 180 insertions(+), 12 deletions(-) diff --git a/lints/0031_anon_dynamic_sql_function_executable.sql b/lints/0031_anon_dynamic_sql_function_executable.sql index b5fd7203..6e2c5d32 100644 --- a/lints/0031_anon_dynamic_sql_function_executable.sql +++ b/lints/0031_anon_dynamic_sql_function_executable.sql @@ -25,7 +25,19 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + -- body with comments removed and string literals blanked out, so + -- keywords and semicolons inside literals are not mistaken for code + regexp_replace( + regexp_replace( + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ) as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -78,9 +90,10 @@ escaped as ( cleaned as ( select e.*, - -- ignore `USING` bound values and quote_*(param) wrappers + -- ignore `USING` bound values (literals are already blanked, so + -- this only matches a real USING clause) and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), + regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' diff --git a/lints/0032_authenticated_dynamic_sql_function_executable.sql b/lints/0032_authenticated_dynamic_sql_function_executable.sql index 886be63a..b99fb43e 100644 --- a/lints/0032_authenticated_dynamic_sql_function_executable.sql +++ b/lints/0032_authenticated_dynamic_sql_function_executable.sql @@ -25,7 +25,19 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + -- body with comments removed and string literals blanked out, so + -- keywords and semicolons inside literals are not mistaken for code + regexp_replace( + regexp_replace( + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ) as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -78,9 +90,10 @@ escaped as ( cleaned as ( select e.*, - -- ignore `USING` bound values and quote_*(param) wrappers + -- ignore `USING` bound values (literals are already blanked, so + -- this only matches a real USING clause) and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), + regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' diff --git a/splinter.sql b/splinter.sql index 1f7c67a6..ced7427f 100644 --- a/splinter.sql +++ b/splinter.sql @@ -1902,7 +1902,19 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + -- body with comments removed and string literals blanked out, so + -- keywords and semicolons inside literals are not mistaken for code + regexp_replace( + regexp_replace( + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ) as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -1955,9 +1967,10 @@ escaped as ( cleaned as ( select e.*, - -- ignore `USING` bound values and quote_*(param) wrappers + -- ignore `USING` bound values (literals are already blanked, so + -- this only matches a real USING clause) and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), + regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' @@ -2068,7 +2081,19 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, + -- body with comments removed and string literals blanked out, so + -- keywords and semicolons inside literals are not mistaken for code + regexp_replace( + regexp_replace( + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ) as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -2121,9 +2146,10 @@ escaped as ( cleaned as ( select e.*, - -- ignore `USING` bound values and quote_*(param) wrappers + -- ignore `USING` bound values (literals are already blanked, so + -- this only matches a real USING clause) and quote_*(param) wrappers regexp_replace( - regexp_replace(e.body, '\musing\s+[^(\x3b\s][^\x3b]*', '', 'g'), + regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', '', 'g' diff --git a/test/expected/0031_anon_dynamic_sql_function_executable.out b/test/expected/0031_anon_dynamic_sql_function_executable.out index 35a0125b..615d3cd8 100644 --- a/test/expected/0031_anon_dynamic_sql_function_executable.out +++ b/test/expected/0031_anon_dynamic_sql_function_executable.out @@ -166,6 +166,36 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_using_in_literal; + ---------------------------------------- + -- POSITIVE (ERROR): the SQL text contains the word `using` before the + -- concatenated parameter (DELETE ... USING, CREATE INDEX ... USING). + -- These are not EXECUTE's own USING clause and must not hide the + -- parameter. The `;` inside the literal must not hide it either. + ---------------------------------------- + create function public.exec_delete_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'delete from public.t using public.u where ' || query; + end $$; + create function public.exec_index_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'select 1; create index on public.t using gin (' || query || ')'; + end $$; + select name, level, metadata->'name' as func_name, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | func_name | cache_key +--------------------------------------+-------+---------------------+-------------------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | "exec_delete_using" | anon_dynamic_sql_function_executable_public_exec_delete_using_query text + anon_dynamic_sql_function_executable | ERROR | "exec_index_using" | anon_dynamic_sql_function_executable_public_exec_index_using_query text +(2 rows) + + rollback to savepoint case_using_in_literal; savepoint case_variadic; ---------------------------------------- -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE diff --git a/test/expected/0032_authenticated_dynamic_sql_function_executable.out b/test/expected/0032_authenticated_dynamic_sql_function_executable.out index 9c5d144d..f6828330 100644 --- a/test/expected/0032_authenticated_dynamic_sql_function_executable.out +++ b/test/expected/0032_authenticated_dynamic_sql_function_executable.out @@ -166,6 +166,36 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_using_in_literal; + ---------------------------------------- + -- POSITIVE (ERROR): the SQL text contains the word `using` before the + -- concatenated parameter (DELETE ... USING, CREATE INDEX ... USING). + -- These are not EXECUTE's own USING clause and must not hide the + -- parameter. The `;` inside the literal must not hide it either. + ---------------------------------------- + create function public.exec_delete_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'delete from public.t using public.u where ' || query; + end $$; + create function public.exec_index_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'select 1; create index on public.t using gin (' || query || ')'; + end $$; + select name, level, metadata->'name' as func_name, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | func_name | cache_key +-----------------------------------------------+-------+---------------------+----------------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | "exec_delete_using" | authenticated_dynamic_sql_function_executable_public_exec_delete_using_query text + authenticated_dynamic_sql_function_executable | ERROR | "exec_index_using" | authenticated_dynamic_sql_function_executable_public_exec_index_using_query text +(2 rows) + + rollback to savepoint case_using_in_literal; savepoint case_variadic; ---------------------------------------- -- POSITIVE (ERROR): VARIADIC text[] parameter used in an EXECUTE diff --git a/test/sql/0031_anon_dynamic_sql_function_executable.sql b/test/sql/0031_anon_dynamic_sql_function_executable.sql index 33895bcd..29eaba9d 100644 --- a/test/sql/0031_anon_dynamic_sql_function_executable.sql +++ b/test/sql/0031_anon_dynamic_sql_function_executable.sql @@ -147,6 +147,34 @@ begin; rollback to savepoint case_unnamed; + savepoint case_using_in_literal; + + ---------------------------------------- + -- POSITIVE (ERROR): the SQL text contains the word `using` before the + -- concatenated parameter (DELETE ... USING, CREATE INDEX ... USING). + -- These are not EXECUTE's own USING clause and must not hide the + -- parameter. The `;` inside the literal must not hide it either. + ---------------------------------------- + create function public.exec_delete_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'delete from public.t using public.u where ' || query; + end $$; + create function public.exec_index_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'select 1; create index on public.t using gin (' || query || ')'; + end $$; + + select name, level, metadata->'name' as func_name, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_using_in_literal; + savepoint case_variadic; ---------------------------------------- diff --git a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql index 7eaa7d48..4c6cff96 100644 --- a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql +++ b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql @@ -147,6 +147,34 @@ begin; rollback to savepoint case_unnamed; + savepoint case_using_in_literal; + + ---------------------------------------- + -- POSITIVE (ERROR): the SQL text contains the word `using` before the + -- concatenated parameter (DELETE ... USING, CREATE INDEX ... USING). + -- These are not EXECUTE's own USING clause and must not hide the + -- parameter. The `;` inside the literal must not hide it either. + ---------------------------------------- + create function public.exec_delete_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'delete from public.t using public.u where ' || query; + end $$; + create function public.exec_index_using(query text) returns void + language plpgsql + security definer + as $$ + begin + execute 'select 1; create index on public.t using gin (' || query || ')'; + end $$; + + select name, level, metadata->'name' as func_name, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_using_in_literal; + savepoint case_variadic; ---------------------------------------- From ffa7a4e61b1a97fb2316b88eacb59307cb2353c4 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 15:10:27 +1300 Subject: [PATCH 5/6] fix: other linting fixes --- ...1_anon_dynamic_sql_function_executable.sql | 102 ++++++--- ...icated_dynamic_sql_function_executable.sql | 102 ++++++--- splinter.sql | 204 +++++++++++++----- ...1_anon_dynamic_sql_function_executable.out | 118 +++++++++- ...icated_dynamic_sql_function_executable.out | 118 +++++++++- ...1_anon_dynamic_sql_function_executable.sql | 100 +++++++++ ...icated_dynamic_sql_function_executable.sql | 100 +++++++++ 7 files changed, 742 insertions(+), 102 deletions(-) diff --git a/lints/0031_anon_dynamic_sql_function_executable.sql b/lints/0031_anon_dynamic_sql_function_executable.sql index 6e2c5d32..b60a8a40 100644 --- a/lints/0031_anon_dynamic_sql_function_executable.sql +++ b/lints/0031_anon_dynamic_sql_function_executable.sql @@ -7,9 +7,10 @@ create view lint."0031_anon_dynamic_sql_function_executable" as -- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement --- (EXECUTE ... USING bound values and quote_ident/quote_literal/ --- quote_nullable(param) are ignored), or +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement, +-- or in an expression assigned to a variable (`v := ... || param`) that +-- is then used in an EXECUTE (EXECUTE ... USING bound values and +-- quote_ident/quote_literal/quote_nullable(param) are ignored), or -- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- @@ -25,19 +26,7 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - -- body with comments removed and string literals blanked out, so - -- keywords and semicolons inside literals are not mistaken for code - regexp_replace( - regexp_replace( - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), - '\$([a-z_]*?)\$.*?\$\1\$', - '''''', - 'g' - ), - '''([^'']|'''')*''', - '''''', - 'g' - ) as body, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -67,10 +56,17 @@ with candidates as ( text_params as ( select c.*, - lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + ip.pos as arg_pos, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || ip.pos)) as arg_name from candidates c cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + -- position among input arguments, which is what `$N` refers to + cross join lateral ( + select count(*) as pos + from generate_series(1, a.ord::int) g + where coalesce(c.proargmodes[g]::text, 'i') in ('i', 'b', 'v') + ) ip where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') and a.typ in ( @@ -83,23 +79,65 @@ text_params as ( escaped as ( select tp.*, - regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + -- match the parameter by name or by position (`$N` also works for + -- named parameters in PL/pgSQL) + '(?:' || regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') || '|\$' || tp.arg_pos || ')' as arg_re from text_params tp ), -cleaned as ( +formatted as ( select e.*, - -- ignore `USING` bound values (literals are already blanked, so - -- this only matches a real USING clause) and quote_*(param) wrappers + -- a parameter passed to format() in a %I or %L slot is quoted, so + -- it is replaced with `null` (like quote_ident/quote_literal). + -- Only format() calls with simple, comma-free arguments and + -- sequential specifiers (no `%1$s`) are recognised; anything else + -- is left in place and counts as a use of the parameter. regexp_replace( - regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), - 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + e.body, + '(?<=(?:' || ( + select string_agg( + 'format\s*\(\s*''(?:[^''%]|%%|'''')*(?:%[-0-9]*[sil](?:[^''%]|%%|'''')*){' || (k - 1) || '}%[-0-9]*[il](?:[^'']|'''')*''\s*(?:,[^,()]+){' || (k - 1) || '},\s*', + '|' + ) + from generate_series(1, 8) k + ) || '))' || e.arg_re || '(?=\s*[,)])', + 'null', + 'g' + ) as fmt_body + from + escaped e +), +cleaned as ( + select + f.*, + -- string literals blanked out (so keywords and semicolons inside + -- them are not mistaken for code), `USING` bound values removed + -- (only a real USING clause is left to match), and quote_*(param) + -- wrappers removed + regexp_replace( + regexp_replace( + regexp_replace( + regexp_replace( + f.fmt_body, + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ), + '\musing\s[^\x3b]*', + '', + 'g' + ), + 'quote_(ident|literal|nullable)\s*\(\s*' || f.arg_re || '\s*\)', '', 'g' ) as exec_body from - escaped e + formatted f ), matches as ( select @@ -110,7 +148,21 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + ( + -- the parameter is used inside an EXECUTE statement + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') + -- or a variable assigned from the parameter (`v := ... || param` + -- or `declare v text := ... || param`) is used in an EXECUTE + or exists ( + select 1 + from regexp_matches( + cl.exec_body, + '(?:([a-z_][a-z0-9_]*)\s+(?:text|varchar(?:\(\d+\))?|character\s+varying(?:\(\d+\))?)|([a-z_][a-z0-9_]*))\s*:=([^\x3b]*[^a-z0-9_$])?' || cl.arg_re || '([^a-z0-9_]|$)', + 'g' + ) as m + where cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || coalesce(m[1], m[2]) || '([^a-z0-9_]|$)') + ) + ) as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from diff --git a/lints/0032_authenticated_dynamic_sql_function_executable.sql b/lints/0032_authenticated_dynamic_sql_function_executable.sql index b99fb43e..63df83bc 100644 --- a/lints/0032_authenticated_dynamic_sql_function_executable.sql +++ b/lints/0032_authenticated_dynamic_sql_function_executable.sql @@ -7,9 +7,10 @@ create view lint."0032_authenticated_dynamic_sql_function_executable" as -- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement --- (EXECUTE ... USING bound values and quote_ident/quote_literal/ --- quote_nullable(param) are ignored), or +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement, +-- or in an expression assigned to a variable (`v := ... || param`) that +-- is then used in an EXECUTE (EXECUTE ... USING bound values and +-- quote_ident/quote_literal/quote_nullable(param) are ignored), or -- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- @@ -25,19 +26,7 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - -- body with comments removed and string literals blanked out, so - -- keywords and semicolons inside literals are not mistaken for code - regexp_replace( - regexp_replace( - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), - '\$([a-z_]*?)\$.*?\$\1\$', - '''''', - 'g' - ), - '''([^'']|'''')*''', - '''''', - 'g' - ) as body, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -67,10 +56,17 @@ with candidates as ( text_params as ( select c.*, - lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + ip.pos as arg_pos, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || ip.pos)) as arg_name from candidates c cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + -- position among input arguments, which is what `$N` refers to + cross join lateral ( + select count(*) as pos + from generate_series(1, a.ord::int) g + where coalesce(c.proargmodes[g]::text, 'i') in ('i', 'b', 'v') + ) ip where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') and a.typ in ( @@ -83,23 +79,65 @@ text_params as ( escaped as ( select tp.*, - regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + -- match the parameter by name or by position (`$N` also works for + -- named parameters in PL/pgSQL) + '(?:' || regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') || '|\$' || tp.arg_pos || ')' as arg_re from text_params tp ), -cleaned as ( +formatted as ( select e.*, - -- ignore `USING` bound values (literals are already blanked, so - -- this only matches a real USING clause) and quote_*(param) wrappers + -- a parameter passed to format() in a %I or %L slot is quoted, so + -- it is replaced with `null` (like quote_ident/quote_literal). + -- Only format() calls with simple, comma-free arguments and + -- sequential specifiers (no `%1$s`) are recognised; anything else + -- is left in place and counts as a use of the parameter. regexp_replace( - regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), - 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + e.body, + '(?<=(?:' || ( + select string_agg( + 'format\s*\(\s*''(?:[^''%]|%%|'''')*(?:%[-0-9]*[sil](?:[^''%]|%%|'''')*){' || (k - 1) || '}%[-0-9]*[il](?:[^'']|'''')*''\s*(?:,[^,()]+){' || (k - 1) || '},\s*', + '|' + ) + from generate_series(1, 8) k + ) || '))' || e.arg_re || '(?=\s*[,)])', + 'null', + 'g' + ) as fmt_body + from + escaped e +), +cleaned as ( + select + f.*, + -- string literals blanked out (so keywords and semicolons inside + -- them are not mistaken for code), `USING` bound values removed + -- (only a real USING clause is left to match), and quote_*(param) + -- wrappers removed + regexp_replace( + regexp_replace( + regexp_replace( + regexp_replace( + f.fmt_body, + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ), + '\musing\s[^\x3b]*', + '', + 'g' + ), + 'quote_(ident|literal|nullable)\s*\(\s*' || f.arg_re || '\s*\)', '', 'g' ) as exec_body from - escaped e + formatted f ), matches as ( select @@ -110,7 +148,21 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + ( + -- the parameter is used inside an EXECUTE statement + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') + -- or a variable assigned from the parameter (`v := ... || param` + -- or `declare v text := ... || param`) is used in an EXECUTE + or exists ( + select 1 + from regexp_matches( + cl.exec_body, + '(?:([a-z_][a-z0-9_]*)\s+(?:text|varchar(?:\(\d+\))?|character\s+varying(?:\(\d+\))?)|([a-z_][a-z0-9_]*))\s*:=([^\x3b]*[^a-z0-9_$])?' || cl.arg_re || '([^a-z0-9_]|$)', + 'g' + ) as m + where cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || coalesce(m[1], m[2]) || '([^a-z0-9_]|$)') + ) + ) as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from diff --git a/splinter.sql b/splinter.sql index ced7427f..dd36e579 100644 --- a/splinter.sql +++ b/splinter.sql @@ -1884,9 +1884,10 @@ union all -- function owner (SECURITY DEFINER) or as `anon` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement --- (EXECUTE ... USING bound values and quote_ident/quote_literal/ --- quote_nullable(param) are ignored), or +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement, +-- or in an expression assigned to a variable (`v := ... || param`) that +-- is then used in an EXECUTE (EXECUTE ... USING bound values and +-- quote_ident/quote_literal/quote_nullable(param) are ignored), or -- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- @@ -1902,19 +1903,7 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - -- body with comments removed and string literals blanked out, so - -- keywords and semicolons inside literals are not mistaken for code - regexp_replace( - regexp_replace( - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), - '\$([a-z_]*?)\$.*?\$\1\$', - '''''', - 'g' - ), - '''([^'']|'''')*''', - '''''', - 'g' - ) as body, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -1944,10 +1933,17 @@ with candidates as ( text_params as ( select c.*, - lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + ip.pos as arg_pos, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || ip.pos)) as arg_name from candidates c cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + -- position among input arguments, which is what `$N` refers to + cross join lateral ( + select count(*) as pos + from generate_series(1, a.ord::int) g + where coalesce(c.proargmodes[g]::text, 'i') in ('i', 'b', 'v') + ) ip where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') and a.typ in ( @@ -1960,23 +1956,65 @@ text_params as ( escaped as ( select tp.*, - regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + -- match the parameter by name or by position (`$N` also works for + -- named parameters in PL/pgSQL) + '(?:' || regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') || '|\$' || tp.arg_pos || ')' as arg_re from text_params tp ), -cleaned as ( +formatted as ( select e.*, - -- ignore `USING` bound values (literals are already blanked, so - -- this only matches a real USING clause) and quote_*(param) wrappers + -- a parameter passed to format() in a %I or %L slot is quoted, so + -- it is replaced with `null` (like quote_ident/quote_literal). + -- Only format() calls with simple, comma-free arguments and + -- sequential specifiers (no `%1$s`) are recognised anything else + -- is left in place and counts as a use of the parameter. regexp_replace( - regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), - 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + e.body, + '(?<=(?:' || ( + select string_agg( + 'format\s*\(\s*''(?:[^''%]|%%|'''')*(?:%[-0-9]*[sil](?:[^''%]|%%|'''')*){' || (k - 1) || '}%[-0-9]*[il](?:[^'']|'''')*''\s*(?:,[^,()]+){' || (k - 1) || '},\s*', + '|' + ) + from generate_series(1, 8) k + ) || '))' || e.arg_re || '(?=\s*[,)])', + 'null', + 'g' + ) as fmt_body + from + escaped e +), +cleaned as ( + select + f.*, + -- string literals blanked out (so keywords and semicolons inside + -- them are not mistaken for code), `USING` bound values removed + -- (only a real USING clause is left to match), and quote_*(param) + -- wrappers removed + regexp_replace( + regexp_replace( + regexp_replace( + regexp_replace( + f.fmt_body, + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ), + '\musing\s[^\x3b]*', + '', + 'g' + ), + 'quote_(ident|literal|nullable)\s*\(\s*' || f.arg_re || '\s*\)', '', 'g' ) as exec_body from - escaped e + formatted f ), matches as ( select @@ -1987,7 +2025,21 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + ( + -- the parameter is used inside an EXECUTE statement + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') + -- or a variable assigned from the parameter (`v := ... || param` + -- or `declare v text := ... || param`) is used in an EXECUTE + or exists ( + select 1 + from regexp_matches( + cl.exec_body, + '(?:([a-z_][a-z0-9_]*)\s+(?:text|varchar(?:\(\d+\))?|character\s+varying(?:\(\d+\))?)|([a-z_][a-z0-9_]*))\s*:=([^\x3b]*[^a-z0-9_$])?' || cl.arg_re || '([^a-z0-9_]|$)', + 'g' + ) as m + where cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || coalesce(m[1], m[2]) || '([^a-z0-9_]|$)') + ) + ) as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from @@ -2063,9 +2115,10 @@ union all -- function owner (SECURITY DEFINER) or as `authenticated` (SECURITY INVOKER). -- -- A function is flagged when either: --- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement --- (EXECUTE ... USING bound values and quote_ident/quote_literal/ --- quote_nullable(param) are ignored), or +-- 1. a text/varchar parameter (or array of either) is used inside an EXECUTE statement, +-- or in an expression assigned to a variable (`v := ... || param`) that +-- is then used in an EXECUTE (EXECUTE ... USING bound values and +-- quote_ident/quote_literal/quote_nullable(param) are ignored), or -- 2. a text/varchar parameter (or array of either) has a SQL-like name (sql, query, stmt, -- ...) and the body contains an EXECUTE anywhere. -- @@ -2081,19 +2134,7 @@ with candidates as ( l.lanname as function_language, p.prosecdef as is_security_definer, pg_catalog.pg_get_userbyid(p.proowner) as owner_name, - -- body with comments removed and string literals blanked out, so - -- keywords and semicolons inside literals are not mistaken for code - regexp_replace( - regexp_replace( - regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g'), - '\$([a-z_]*?)\$.*?\$\1\$', - '''''', - 'g' - ), - '''([^'']|'''')*''', - '''''', - 'g' - ) as body, + regexp_replace(lower(p.prosrc), '--[^\n]*', '', 'g') as body, p.proargnames, p.proargmodes, coalesce(p.proallargtypes, p.proargtypes::pg_catalog.oid[]) as all_arg_types @@ -2123,10 +2164,17 @@ with candidates as ( text_params as ( select c.*, - lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || a.ord)) as arg_name + ip.pos as arg_pos, + lower(coalesce(nullif(c.proargnames[a.ord], ''), '$' || ip.pos)) as arg_name from candidates c cross join lateral unnest(c.all_arg_types) with ordinality as a(typ, ord) + -- position among input arguments, which is what `$N` refers to + cross join lateral ( + select count(*) as pos + from generate_series(1, a.ord::int) g + where coalesce(c.proargmodes[g]::text, 'i') in ('i', 'b', 'v') + ) ip where coalesce(c.proargmodes[a.ord]::text, 'i') in ('i', 'b', 'v') and a.typ in ( @@ -2139,23 +2187,65 @@ text_params as ( escaped as ( select tp.*, - regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') as arg_re + -- match the parameter by name or by position (`$N` also works for + -- named parameters in PL/pgSQL) + '(?:' || regexp_replace(tp.arg_name, '([^a-z0-9_])', '\\\1', 'g') || '|\$' || tp.arg_pos || ')' as arg_re from text_params tp ), -cleaned as ( +formatted as ( select e.*, - -- ignore `USING` bound values (literals are already blanked, so - -- this only matches a real USING clause) and quote_*(param) wrappers + -- a parameter passed to format() in a %I or %L slot is quoted, so + -- it is replaced with `null` (like quote_ident/quote_literal). + -- Only format() calls with simple, comma-free arguments and + -- sequential specifiers (no `%1$s`) are recognised anything else + -- is left in place and counts as a use of the parameter. regexp_replace( - regexp_replace(e.body, '\musing\s[^\x3b]*', '', 'g'), - 'quote_(ident|literal|nullable)\s*\(\s*' || e.arg_re || '\s*\)', + e.body, + '(?<=(?:' || ( + select string_agg( + 'format\s*\(\s*''(?:[^''%]|%%|'''')*(?:%[-0-9]*[sil](?:[^''%]|%%|'''')*){' || (k - 1) || '}%[-0-9]*[il](?:[^'']|'''')*''\s*(?:,[^,()]+){' || (k - 1) || '},\s*', + '|' + ) + from generate_series(1, 8) k + ) || '))' || e.arg_re || '(?=\s*[,)])', + 'null', + 'g' + ) as fmt_body + from + escaped e +), +cleaned as ( + select + f.*, + -- string literals blanked out (so keywords and semicolons inside + -- them are not mistaken for code), `USING` bound values removed + -- (only a real USING clause is left to match), and quote_*(param) + -- wrappers removed + regexp_replace( + regexp_replace( + regexp_replace( + regexp_replace( + f.fmt_body, + '\$([a-z_]*?)\$.*?\$\1\$', + '''''', + 'g' + ), + '''([^'']|'''')*''', + '''''', + 'g' + ), + '\musing\s[^\x3b]*', + '', + 'g' + ), + 'quote_(ident|literal|nullable)\s*\(\s*' || f.arg_re || '\s*\)', '', 'g' ) as exec_body from - escaped e + formatted f ), matches as ( select @@ -2166,7 +2256,21 @@ matches as ( cl.is_security_definer, cl.owner_name, cl.arg_name, - cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') as direct_use, + ( + -- the parameter is used inside an EXECUTE statement + cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || cl.arg_re || '([^a-z0-9_]|$)') + -- or a variable assigned from the parameter (`v := ... || param` + -- or `declare v text := ... || param`) is used in an EXECUTE + or exists ( + select 1 + from regexp_matches( + cl.exec_body, + '(?:([a-z_][a-z0-9_]*)\s+(?:text|varchar(?:\(\d+\))?|character\s+varying(?:\(\d+\))?)|([a-z_][a-z0-9_]*))\s*:=([^\x3b]*[^a-z0-9_$])?' || cl.arg_re || '([^a-z0-9_]|$)', + 'g' + ) as m + where cl.exec_body ~ ('\mexecute\M[^\x3b]*[^a-z0-9_$]' || coalesce(m[1], m[2]) || '([^a-z0-9_]|$)') + ) + ) as direct_use, cl.arg_name in ('sql', 'query', 'queries', 'stmt', 'statement', 'command', 'cmd', 'sql_text', 'sql_query', 'sql_string', 'query_text', 'query_string', 'raw_sql', 'raw_query') and cl.exec_body ~ '\mexecute\M' as renamed_use from diff --git a/test/expected/0031_anon_dynamic_sql_function_executable.out b/test/expected/0031_anon_dynamic_sql_function_executable.out index 615d3cd8..c3859bbe 100644 --- a/test/expected/0031_anon_dynamic_sql_function_executable.out +++ b/test/expected/0031_anon_dynamic_sql_function_executable.out @@ -67,6 +67,70 @@ begin; ------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- (0 rows) + -- NEGATIVE: the parameter is only bound with USING; the variable holds + -- static SQL + create function public.static_var(p_name text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from auth.users where email = $1'; r int; + begin + execute q into r using p_name; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: the parameter is quoted before it is put in a variable + create function public.quoted_var(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || quote_ident(p_table); + execute q into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: parameters passed to format() only in %I / %L slots + create function public.fmt_quoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %L', p_table, p_val) into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- POSITIVE (ERROR): the second argument goes into a %s slot + create function public.fmt_unquoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %s', p_table, p_val) into r; + return r; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +--------------------------------------+-------+----------------+----------------------------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | ["p_val"] | anon_dynamic_sql_function_executable_public_fmt_unquoted_p_table text, p_val text +(1 row) + rollback to savepoint a; savepoint case_definer; ---------------------------------------- @@ -143,7 +207,7 @@ begin; from lint."0031_anon_dynamic_sql_function_executable"; name | level | sql_parameters | cache_key --------------------------------------+-------+----------------+--------------------------------------------------------------------- - anon_dynamic_sql_function_executable | WARN | ["sql"] | anon_dynamic_sql_function_executable_public_insert_entries_sql text + anon_dynamic_sql_function_executable | ERROR | ["sql"] | anon_dynamic_sql_function_executable_public_insert_entries_sql text (1 row) rollback to savepoint case_renamed; @@ -166,6 +230,58 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_positional; + ---------------------------------------- + -- POSITIVE (ERROR): named parameter referenced as $1 in the EXECUTE + ---------------------------------------- + create function public.run_positional(p text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +--------------------------------------+-------+----------------+------------------------------------------------------------------- + anon_dynamic_sql_function_executable | ERROR | ["p"] | anon_dynamic_sql_function_executable_public_run_positional_p text +(1 row) + + rollback to savepoint case_positional; + savepoint case_variable; + ---------------------------------------- + -- POSITIVE (ERROR): the parameter is built into a local variable, either + -- by assignment or in the declaration, and the variable is EXECUTEd. + ---------------------------------------- + create function public.count_rows(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || p_table; + execute q into r; + return r; + end $$; + create function public.count_rows_declared(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from ' || p_table; r int; + begin + execute q into r; + return r; + end $$; + select name, level, metadata->'name' as func_name, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + name | level | func_name | cache_key +--------------------------------------+-------+-----------------------+------------------------------------------------------------------------------ + anon_dynamic_sql_function_executable | ERROR | "count_rows" | anon_dynamic_sql_function_executable_public_count_rows_p_table text + anon_dynamic_sql_function_executable | ERROR | "count_rows_declared" | anon_dynamic_sql_function_executable_public_count_rows_declared_p_table text +(2 rows) + + rollback to savepoint case_variable; savepoint case_using_in_literal; ---------------------------------------- -- POSITIVE (ERROR): the SQL text contains the word `using` before the diff --git a/test/expected/0032_authenticated_dynamic_sql_function_executable.out b/test/expected/0032_authenticated_dynamic_sql_function_executable.out index f6828330..f3e73463 100644 --- a/test/expected/0032_authenticated_dynamic_sql_function_executable.out +++ b/test/expected/0032_authenticated_dynamic_sql_function_executable.out @@ -67,6 +67,70 @@ begin; ------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- (0 rows) + -- NEGATIVE: the parameter is only bound with USING; the variable holds + -- static SQL + create function public.static_var(p_name text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from auth.users where email = $1'; r int; + begin + execute q into r using p_name; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: the parameter is quoted before it is put in a variable + create function public.quoted_var(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || quote_ident(p_table); + execute q into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- NEGATIVE: parameters passed to format() only in %I / %L slots + create function public.fmt_quoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %L', p_table, p_val) into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + name | title | level | facing | categories | description | detail | remediation | metadata | cache_key +------+-------+-------+--------+------------+-------------+--------+-------------+----------+----------- +(0 rows) + + -- POSITIVE (ERROR): the second argument goes into a %s slot + create function public.fmt_unquoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %s', p_table, p_val) into r; + return r; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +-----------------------------------------------+-------+----------------+-------------------------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | ["p_val"] | authenticated_dynamic_sql_function_executable_public_fmt_unquoted_p_table text, p_val text +(1 row) + rollback to savepoint a; savepoint case_definer; ---------------------------------------- @@ -143,7 +207,7 @@ begin; from lint."0032_authenticated_dynamic_sql_function_executable"; name | level | sql_parameters | cache_key -----------------------------------------------+-------+----------------+------------------------------------------------------------------------------ - authenticated_dynamic_sql_function_executable | WARN | ["sql"] | authenticated_dynamic_sql_function_executable_public_insert_entries_sql text + authenticated_dynamic_sql_function_executable | ERROR | ["sql"] | authenticated_dynamic_sql_function_executable_public_insert_entries_sql text (1 row) rollback to savepoint case_renamed; @@ -166,6 +230,58 @@ begin; (1 row) rollback to savepoint case_unnamed; + savepoint case_positional; + ---------------------------------------- + -- POSITIVE (ERROR): named parameter referenced as $1 in the EXECUTE + ---------------------------------------- + create function public.run_positional(p text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | sql_parameters | cache_key +-----------------------------------------------+-------+----------------+---------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | ["p"] | authenticated_dynamic_sql_function_executable_public_run_positional_p text +(1 row) + + rollback to savepoint case_positional; + savepoint case_variable; + ---------------------------------------- + -- POSITIVE (ERROR): the parameter is built into a local variable, either + -- by assignment or in the declaration, and the variable is EXECUTEd. + ---------------------------------------- + create function public.count_rows(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || p_table; + execute q into r; + return r; + end $$; + create function public.count_rows_declared(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from ' || p_table; r int; + begin + execute q into r; + return r; + end $$; + select name, level, metadata->'name' as func_name, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + name | level | func_name | cache_key +-----------------------------------------------+-------+-----------------------+--------------------------------------------------------------------------------------- + authenticated_dynamic_sql_function_executable | ERROR | "count_rows" | authenticated_dynamic_sql_function_executable_public_count_rows_p_table text + authenticated_dynamic_sql_function_executable | ERROR | "count_rows_declared" | authenticated_dynamic_sql_function_executable_public_count_rows_declared_p_table text +(2 rows) + + rollback to savepoint case_variable; savepoint case_using_in_literal; ---------------------------------------- -- POSITIVE (ERROR): the SQL text contains the word `using` before the diff --git a/test/sql/0031_anon_dynamic_sql_function_executable.sql b/test/sql/0031_anon_dynamic_sql_function_executable.sql index 29eaba9d..0c44b265 100644 --- a/test/sql/0031_anon_dynamic_sql_function_executable.sql +++ b/test/sql/0031_anon_dynamic_sql_function_executable.sql @@ -54,6 +54,57 @@ begin; end $$; select * from lint."0031_anon_dynamic_sql_function_executable"; + -- NEGATIVE: the parameter is only bound with USING; the variable holds + -- static SQL + create function public.static_var(p_name text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from auth.users where email = $1'; r int; + begin + execute q into r using p_name; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- NEGATIVE: the parameter is quoted before it is put in a variable + create function public.quoted_var(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || quote_ident(p_table); + execute q into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- NEGATIVE: parameters passed to format() only in %I / %L slots + create function public.fmt_quoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %L', p_table, p_val) into r; + return r; + end $$; + select * from lint."0031_anon_dynamic_sql_function_executable"; + + -- POSITIVE (ERROR): the second argument goes into a %s slot + create function public.fmt_unquoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %s', p_table, p_val) into r; + return r; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + rollback to savepoint a; savepoint case_definer; @@ -147,6 +198,55 @@ begin; rollback to savepoint case_unnamed; + savepoint case_positional; + + ---------------------------------------- + -- POSITIVE (ERROR): named parameter referenced as $1 in the EXECUTE + ---------------------------------------- + create function public.run_positional(p text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_positional; + + savepoint case_variable; + + ---------------------------------------- + -- POSITIVE (ERROR): the parameter is built into a local variable, either + -- by assignment or in the declaration, and the variable is EXECUTEd. + ---------------------------------------- + create function public.count_rows(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || p_table; + execute q into r; + return r; + end $$; + create function public.count_rows_declared(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from ' || p_table; r int; + begin + execute q into r; + return r; + end $$; + + select name, level, metadata->'name' as func_name, cache_key + from lint."0031_anon_dynamic_sql_function_executable"; + + rollback to savepoint case_variable; + savepoint case_using_in_literal; ---------------------------------------- diff --git a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql index 4c6cff96..66c3fc6e 100644 --- a/test/sql/0032_authenticated_dynamic_sql_function_executable.sql +++ b/test/sql/0032_authenticated_dynamic_sql_function_executable.sql @@ -54,6 +54,57 @@ begin; end $$; select * from lint."0032_authenticated_dynamic_sql_function_executable"; + -- NEGATIVE: the parameter is only bound with USING; the variable holds + -- static SQL + create function public.static_var(p_name text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from auth.users where email = $1'; r int; + begin + execute q into r using p_name; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- NEGATIVE: the parameter is quoted before it is put in a variable + create function public.quoted_var(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || quote_ident(p_table); + execute q into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- NEGATIVE: parameters passed to format() only in %I / %L slots + create function public.fmt_quoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %L', p_table, p_val) into r; + return r; + end $$; + select * from lint."0032_authenticated_dynamic_sql_function_executable"; + + -- POSITIVE (ERROR): the second argument goes into a %s slot + create function public.fmt_unquoted(p_table text, p_val text) returns int + language plpgsql + security definer + as $$ + declare r int; + begin + execute format('select count(*)::int from %I where name = %s', p_table, p_val) into r; + return r; + end $$; + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + rollback to savepoint a; savepoint case_definer; @@ -147,6 +198,55 @@ begin; rollback to savepoint case_unnamed; + savepoint case_positional; + + ---------------------------------------- + -- POSITIVE (ERROR): named parameter referenced as $1 in the EXECUTE + ---------------------------------------- + create function public.run_positional(p text) returns void + language plpgsql + security definer + as $$ + begin + execute $1; + end $$; + + select name, level, metadata->'sql_parameters' as sql_parameters, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_positional; + + savepoint case_variable; + + ---------------------------------------- + -- POSITIVE (ERROR): the parameter is built into a local variable, either + -- by assignment or in the declaration, and the variable is EXECUTEd. + ---------------------------------------- + create function public.count_rows(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text; r int; + begin + q := 'select count(*)::int from ' || p_table; + execute q into r; + return r; + end $$; + create function public.count_rows_declared(p_table text) returns int + language plpgsql + security definer + as $$ + declare q text := 'select count(*)::int from ' || p_table; r int; + begin + execute q into r; + return r; + end $$; + + select name, level, metadata->'name' as func_name, cache_key + from lint."0032_authenticated_dynamic_sql_function_executable"; + + rollback to savepoint case_variable; + savepoint case_using_in_literal; ---------------------------------------- From acd7f4ea2e3969235676ac43e9b6eb3bbb1e6bf2 Mon Sep 17 00:00:00 2001 From: Stephen Morgan Date: Thu, 12 Nov 2026 15:11:54 +1300 Subject: [PATCH 6/6] fix: doc accuracy --- docs/0031_anon_dynamic_sql_function_executable.md | 2 +- docs/0032_authenticated_dynamic_sql_function_executable.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/0031_anon_dynamic_sql_function_executable.md b/docs/0031_anon_dynamic_sql_function_executable.md index e0054a7e..19a906f2 100644 --- a/docs/0031_anon_dynamic_sql_function_executable.md +++ b/docs/0031_anon_dynamic_sql_function_executable.md @@ -2,7 +2,7 @@ **Summary:** This function is callable without signing in and appears to `EXECUTE` a SQL string passed in by the caller. -**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so anyone with the public anon key can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS (level `ERROR`). If it is `SECURITY INVOKER` the SQL runs as `anon` and is limited by that role's grants and RLS (level `WARN`). Either way the caller can read your schema and any data `anon` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. +**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so anyone with the public anon key can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS. If it is `SECURITY INVOKER` the SQL runs as `anon` and is limited by that role's grants and RLS. The level is `ERROR` for a `SECURITY DEFINER` function whose parameter appears directly in an `EXECUTE`, and `WARN` for everything else, including `SECURITY INVOKER` functions and functions matched only because a parameter has a SQL-like name. Either way the caller can read your schema and any data `anon` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. > **See also: lint [0032_authenticated_dynamic_sql_function_executable](0032_authenticated_dynamic_sql_function_executable.md).** It is the same check for signed-in users. Revoking from `anon` alone usually leaves the function callable by every signed-up user, so address both together. Lint [0028_anon_security_definer_function_executable](0028_anon_security_definer_function_executable.md) covers `SECURITY DEFINER` functions exposed to `anon` in general. diff --git a/docs/0032_authenticated_dynamic_sql_function_executable.md b/docs/0032_authenticated_dynamic_sql_function_executable.md index a87ab534..c682cb18 100644 --- a/docs/0032_authenticated_dynamic_sql_function_executable.md +++ b/docs/0032_authenticated_dynamic_sql_function_executable.md @@ -2,7 +2,7 @@ **Summary:** This function is callable by signed-in users and appears to `EXECUTE` a SQL string passed in by the caller. -**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so any signed-in user (with open signup, anyone with a throwaway account) can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS (level `ERROR`). If it is `SECURITY INVOKER` the SQL runs as `authenticated` and is limited by that role's grants and RLS (level `WARN`). Either way the caller can read your schema and any data `authenticated` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. +**Ramification:** The function is reachable through `POST /rest/v1/rpc/`, so any signed-in user (with open signup, anyone with a throwaway account) can run arbitrary SQL through it. If the function is `SECURITY DEFINER` the SQL runs as the function owner and bypasses RLS. If it is `SECURITY INVOKER` the SQL runs as `authenticated` and is limited by that role's grants and RLS. The level is `ERROR` for a `SECURITY DEFINER` function whose parameter appears directly in an `EXECUTE`, and `WARN` for everything else, including `SECURITY INVOKER` functions and functions matched only because a parameter has a SQL-like name. Either way the caller can read your schema and any data `authenticated` can reach. Revoke `EXECUTE`, or replace the function with fixed operations. > **See also: lint [0031_anon_dynamic_sql_function_executable](0031_anon_dynamic_sql_function_executable.md).** It is the same check for `anon`. Revoking from `authenticated` alone can leave the function callable without signing in, so address both together. Lint [0029_authenticated_security_definer_function_executable](0029_authenticated_security_definer_function_executable.md) covers `SECURITY DEFINER` functions exposed to `authenticated` in general.