From 0a73bdd1b1f0571b28cb168e8b16678d1e6bfbb0 Mon Sep 17 00:00:00 2001 From: Devansh Saxena Date: Wed, 10 Jul 2024 18:26:44 +0530 Subject: [PATCH] [#14419] YSQL: import Ban role pg_signal_backend from more superuser backend types. Summary: Import upstream postgres commit e082734c8e78e6622a0422e612a870278721e83f from REL_11_STABLE. This is needed as a mitigation for CVE-2023-5870. Original commit message: ``` Ban role pg_signal_backend from more superuser backend types. Documentation says it cannot signal "a backend owned by a superuser". On the contrary, it could signal background workers, including the logical replication launcher. It could signal autovacuum workers and the autovacuum launcher. Block all that. Signaling autovacuum workers and those two launchers doesn't stall progress beyond what one could achieve other ways. If a cluster uses a non-core extension with a background worker that does not auto-restart, this could create a denial of service with respect to that background worker. A background worker with bugs in its code for responding to terminations or cancellations could experience those bugs at a time the pg_signal_backend member chooses. Back-patch to v11 (all supported versions). Reviewed by Jelte Fennema-Nio. Reported by Hemanth Sandrana and Mahendrakar Srinivasarao. Security: CVE-2023-5870 ``` Jira: DB-3834 Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgRegressAuthorization#testPgRegressAuthorization Reviewers: jason, skumar Reviewed By: jason Subscribers: jason, yql Differential Revision: https://phorge.dev.yugabyte.com/D32400 --- src/postgres/src/backend/utils/adt/misc.c | 9 +++++++-- .../src/test/regress/expected/privileges.out | 18 ++++++++++++++++++ .../test/regress/expected/yb_pg_privileges.out | 18 ++++++++++++++++++ .../src/test/regress/sql/privileges.sql | 15 +++++++++++++++ .../src/test/regress/sql/yb_pg_privileges.sql | 15 +++++++++++++++ 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/postgres/src/backend/utils/adt/misc.c b/src/postgres/src/backend/utils/adt/misc.c index 3963bb53ad72..87e507a2d898 100644 --- a/src/postgres/src/backend/utils/adt/misc.c +++ b/src/postgres/src/backend/utils/adt/misc.c @@ -241,8 +241,13 @@ pg_signal_backend(int pid, int sig) return SIGNAL_BACKEND_ERROR; } - /* Only allow superusers to signal superuser-owned backends. */ - if (superuser_arg(proc->roleId) && !superuser()) + /* + * Only allow superusers to signal superuser-owned backends. Any process + * not advertising a role might have the importance of a superuser-owned + * backend, so treat it that way. + */ + if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId)) && + !superuser()) return SIGNAL_BACKEND_NOSUPERUSER; /* Users can signal backends they have role membership in. */ diff --git a/src/postgres/src/test/regress/expected/privileges.out b/src/postgres/src/test/regress/expected/privileges.out index a66b5f853a72..559db6311068 100644 --- a/src/postgres/src/test/regress/expected/privileges.out +++ b/src/postgres/src/test/regress/expected/privileges.out @@ -1646,6 +1646,24 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied ERROR: permission denied for table pg_largeobject +-- pg_signal_backend can't signal superusers +RESET SESSION AUTHORIZATION; +BEGIN; +CREATE OR REPLACE FUNCTION terminate_nothrow(pid int) RETURNS bool + LANGUAGE plpgsql SECURITY DEFINER SET client_min_messages = error AS $$ +BEGIN + RETURN pg_terminate_backend($1); +EXCEPTION WHEN OTHERS THEN + RETURN false; +END$$; +ALTER FUNCTION terminate_nothrow OWNER TO pg_signal_backend; +SELECT backend_type FROM pg_stat_activity +WHERE CASE WHEN COALESCE(usesysid, 10) = 10 THEN terminate_nothrow(pid) END; + backend_type +-------------- +(0 rows) + +ROLLBACK; -- test default ACLs \c - CREATE SCHEMA testns; diff --git a/src/postgres/src/test/regress/expected/yb_pg_privileges.out b/src/postgres/src/test/regress/expected/yb_pg_privileges.out index 05892fcaf0eb..2744f4831ee6 100644 --- a/src/postgres/src/test/regress/expected/yb_pg_privileges.out +++ b/src/postgres/src/test/regress/expected/yb_pg_privileges.out @@ -1603,6 +1603,24 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied ERROR: permission denied for table pg_largeobject +-- pg_signal_backend can't signal superusers +RESET SESSION AUTHORIZATION; +BEGIN; +CREATE OR REPLACE FUNCTION terminate_nothrow(pid int) RETURNS bool + LANGUAGE plpgsql SECURITY DEFINER SET client_min_messages = error AS $$ +BEGIN + RETURN pg_terminate_backend($1); +EXCEPTION WHEN OTHERS THEN + RETURN false; +END$$; +ALTER FUNCTION terminate_nothrow OWNER TO pg_signal_backend; +SELECT backend_type FROM pg_stat_activity +WHERE CASE WHEN COALESCE(usesysid, 10) = 10 THEN terminate_nothrow(pid) END; + backend_type +-------------- +(0 rows) + +ROLLBACK; -- test default ACLs \c - CREATE SCHEMA testns; diff --git a/src/postgres/src/test/regress/sql/privileges.sql b/src/postgres/src/test/regress/sql/privileges.sql index 8d27ce401e8c..bbfb22b8ff1c 100644 --- a/src/postgres/src/test/regress/sql/privileges.sql +++ b/src/postgres/src/test/regress/sql/privileges.sql @@ -1030,6 +1030,21 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied +-- pg_signal_backend can't signal superusers +RESET SESSION AUTHORIZATION; +BEGIN; +CREATE OR REPLACE FUNCTION terminate_nothrow(pid int) RETURNS bool + LANGUAGE plpgsql SECURITY DEFINER SET client_min_messages = error AS $$ +BEGIN + RETURN pg_terminate_backend($1); +EXCEPTION WHEN OTHERS THEN + RETURN false; +END$$; +ALTER FUNCTION terminate_nothrow OWNER TO pg_signal_backend; +SELECT backend_type FROM pg_stat_activity +WHERE CASE WHEN COALESCE(usesysid, 10) = 10 THEN terminate_nothrow(pid) END; +ROLLBACK; + -- test default ACLs \c - diff --git a/src/postgres/src/test/regress/sql/yb_pg_privileges.sql b/src/postgres/src/test/regress/sql/yb_pg_privileges.sql index 92401b4b1fb4..774154f350e8 100644 --- a/src/postgres/src/test/regress/sql/yb_pg_privileges.sql +++ b/src/postgres/src/test/regress/sql/yb_pg_privileges.sql @@ -1032,6 +1032,21 @@ SELECT * FROM pg_largeobject LIMIT 0; SET SESSION AUTHORIZATION regress_priv_user1; SELECT * FROM pg_largeobject LIMIT 0; -- to be denied +-- pg_signal_backend can't signal superusers +RESET SESSION AUTHORIZATION; +BEGIN; +CREATE OR REPLACE FUNCTION terminate_nothrow(pid int) RETURNS bool + LANGUAGE plpgsql SECURITY DEFINER SET client_min_messages = error AS $$ +BEGIN + RETURN pg_terminate_backend($1); +EXCEPTION WHEN OTHERS THEN + RETURN false; +END$$; +ALTER FUNCTION terminate_nothrow OWNER TO pg_signal_backend; +SELECT backend_type FROM pg_stat_activity +WHERE CASE WHEN COALESCE(usesysid, 10) = 10 THEN terminate_nothrow(pid) END; +ROLLBACK; + -- test default ACLs \c -