diff --git a/src/supautils.c b/src/supautils.c index 3bfb435..10257b3 100644 --- a/src/supautils.c +++ b/src/supautils.c @@ -227,7 +227,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { // Allow setting bypassrls & replication. switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -278,7 +279,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { bool already_switched_to_superuser = false; switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -367,7 +369,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -482,11 +485,11 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { override_create_ext_statement(stmt, total_epos, epos); if (is_extension_privileged(stmt->extname, privileged_extensions)) { - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); } else { if (!already_switched_to_superuser) { switch_to_original_role(); - already_switched_to_superuser = false; } run_process_utility_hook(prev_hook); @@ -519,7 +522,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -545,7 +549,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -580,7 +585,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); CreateFdwStmt *stmt = (CreateFdwStmt *)utility_stmt; @@ -610,7 +616,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); CreatePublicationStmt *stmt = (CreatePublicationStmt *)utility_stmt; @@ -639,7 +646,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -663,7 +671,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -690,7 +699,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -719,7 +729,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -752,7 +763,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -782,7 +794,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -815,7 +828,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { bool already_switched_to_superuser = false; switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -852,7 +866,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { bool already_switched_to_superuser = false; switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!already_switched_to_superuser) { switch_to_original_role(); @@ -904,7 +919,8 @@ static void supautils_hook(PROCESS_UTILITY_PARAMS) { switch_to_superuser(supautils_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook_with_cleanup( + prev_hook, already_switched_to_superuser, switch_to_original_role); if (!current_user_is_super) // Change event trigger owner to the current role (which is a privileged diff --git a/src/utils.h b/src/utils.h index 9d87d83..0e6b065 100644 --- a/src/utils.h +++ b/src/utils.h @@ -72,6 +72,20 @@ standard_ProcessUtility(PROCESS_UTILITY_ARGS); \ } +#define run_process_utility_hook_with_cleanup(process_utility_hook, \ + already_switched_to_superuser, \ + switch_to_original_role) \ + PG_TRY(); \ + { run_process_utility_hook(prev_hook); } \ + PG_CATCH(); \ + { \ + if (!already_switched_to_superuser) { \ + switch_to_original_role(); \ + } \ + PG_RE_THROW(); \ + } \ + PG_END_TRY(); + // helper for testing a guc config #if TEST # define SUPAUTILS_GUC_CONTEXT_SIGHUP PGC_USERSET diff --git a/test/expected/privileged_role.out b/test/expected/privileged_role.out index 49c274a..6f3cfe7 100644 --- a/test/expected/privileged_role.out +++ b/test/expected/privileged_role.out @@ -246,3 +246,19 @@ drop cascades to function deny_drop_triggers.f() set role privileged_role; \echo +-- regression: is_switched_to_superuser is restored when an exception is thrown +create publication p; +do $$ +begin + alter publication p add table missing_table; +exception + when undefined_table then + null; +end $$; +-- if is_switched_to_superuser is not restored, role switching won't happen here +-- and publication creation would fail +create publication pp for all tables; +drop publication p; +drop publication pp; +\echo + diff --git a/test/sql/privileged_role.sql b/test/sql/privileged_role.sql index 91a3231..1dad58c 100644 --- a/test/sql/privileged_role.sql +++ b/test/sql/privileged_role.sql @@ -189,3 +189,20 @@ set role postgres; drop schema deny_drop_triggers cascade; set role privileged_role; \echo + +-- regression: is_switched_to_superuser is restored when an exception is thrown +create publication p; +do $$ +begin + alter publication p add table missing_table; +exception + when undefined_table then + null; +end $$; +-- if is_switched_to_superuser is not restored, role switching won't happen here +-- and publication creation would fail +create publication pp for all tables; + +drop publication p; +drop publication pp; +\echo