Skip to content

Commit

Permalink
fix: prevent nested switch_to_superuser() from corrupting prev_role_*
Browse files Browse the repository at this point in the history
  • Loading branch information
@soedirgo
soedirgo committed Jan 19, 2024
1 parent bf9c134 commit a335308
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 36 deletions.
11 changes: 9 additions & 2 deletions nix/withTmpDb.sh.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ options="-F -c listen_addresses=\"\" -k $PGDATA -c shared_preload_libraries=\"pg

reserved_roles="supabase_storage_admin, anon, reserved_but_not_yet_created, authenticator*"
reserved_memberships="pg_read_server_files, pg_write_server_files, pg_execute_server_program, role_with_reserved_membership"
privileged_extensions="hstore, postgres_fdw, pg_tle"
privileged_extensions="citext, cube, hstore, postgres_fdw, pg_tle"
privileged_extensions_custom_scripts_path="$tmpdir/privileged_extensions_custom_scripts"
privileged_role="privileged_role"
privileged_role_allowed_configs="session_replication_role, pgrst.*, other.nested.*"
Expand All @@ -27,7 +27,8 @@ cexts_option='-c supautils.constrained_extensions="{\"adminpack\": { \"cpu\": 64

pg_ctl start -o "$options" -o "$reserved_stuff_options" -o "$placeholder_stuff_options" -o "$cexts_option"

mkdir -p "$tmpdir/privileged_extensions_custom_scripts/hstore"
# print notice when creating a TLE
mkdir -p "$tmpdir/privileged_extensions_custom_scripts"
echo "do \$\$
begin
if not exists (select from pg_extension where extname = 'pg_tle') then
Expand All @@ -37,6 +38,12 @@ echo "do \$\$
raise notice 'extname: %, extschema: %, extversion: %, extcascade: %', @extname@, @extschema@, @extversion@, @extcascade@;
end if;
end \$\$;" > "$tmpdir/privileged_extensions_custom_scripts/before-create.sql"

mkdir -p "$tmpdir/privileged_extensions_custom_scripts/citext"
echo 'create extension cube;' > "$tmpdir/privileged_extensions_custom_scripts/citext/after-create.sql"

# assert both before-create and after-create scripts are run
mkdir -p "$tmpdir/privileged_extensions_custom_scripts/hstore"
echo 'create table t1();' > "$tmpdir/privileged_extensions_custom_scripts/hstore/before-create.sql"
echo 'drop table t1; create table t2 as values (1);' > "$tmpdir/privileged_extensions_custom_scripts/hstore/after-create.sql"

Expand Down
6 changes: 3 additions & 3 deletions shell.nix
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
with import (builtins.fetchTarball {
name = "2022-10-25";
url = "https://github.com/NixOS/nixpkgs/archive/a11f8032aa9de58be11190b71320f98f9a3c395b.tar.gz";
sha256 = "101y90kqqfqc5vkigw5rbcqw01cg9nndknz4q4gb28zi4918r1hz";
name = "23.11";
url = "https://github.com/NixOS/nixpkgs/archive/refs/tags/23.11.tar.gz";
sha256 = "1ndiv385w1qyb3b18vw13991fzb9wg4cl21wglk89grsfsnra41k";
}) {};
let
supportedPgVersions = [
Expand Down
48 changes: 36 additions & 12 deletions src/privileged_extensions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ void handle_create_extension(
char *extversion = NULL;
bool extcascade = false;
ListCell *option_cell = NULL;
bool already_switched_to_superuser = false;

foreach (option_cell, stmt->options) {
DefElem *defel = (DefElem *)lfirst(option_cell);
Expand All @@ -120,14 +121,17 @@ void handle_create_extension(
}
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

snprintf(filename, MAXPGPATH, "%s/before-create.sql",
privileged_extensions_custom_scripts_path);
run_custom_script(filename, stmt->extname, extschema, extversion,
extcascade);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
}

// Run per-extension before-create script.
Expand All @@ -139,6 +143,7 @@ void handle_create_extension(
char *extversion = NULL;
bool extcascade = false;
ListCell *option_cell = NULL;
bool already_switched_to_superuser = false;

foreach (option_cell, stmt->options) {
DefElem *defel = (DefElem *)lfirst(option_cell);
Expand All @@ -155,24 +160,31 @@ void handle_create_extension(
}
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

snprintf(filename, MAXPGPATH, "%s/%s/before-create.sql",
privileged_extensions_custom_scripts_path, stmt->extname);
run_custom_script(filename, stmt->extname, extschema, extversion,
extcascade);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
}

// Run `CREATE EXTENSION`.
if (!superuser() && is_string_in_comma_delimited_string(
stmt->extname, privileged_extensions)) {
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

run_process_utility_hook(process_utility_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
} else {
run_process_utility_hook(process_utility_hook);
}
Expand All @@ -186,6 +198,7 @@ void handle_create_extension(
char *extversion = NULL;
bool extcascade = false;
ListCell *option_cell = NULL;
bool already_switched_to_superuser = false;

foreach (option_cell, stmt->options) {
DefElem *defel = (DefElem *)lfirst(option_cell);
Expand All @@ -202,14 +215,17 @@ void handle_create_extension(
}
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

snprintf(filename, MAXPGPATH, "%s/%s/after-create.sql",
privileged_extensions_custom_scripts_path, stmt->extname);
run_custom_script(filename, stmt->extname, extschema, extversion,
extcascade);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
}

pfree(filename);
Expand All @@ -223,11 +239,15 @@ void handle_alter_extension(

if (is_string_in_comma_delimited_string(stmt->extname,
privileged_extensions)) {
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

run_process_utility_hook(process_utility_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
} else {
run_process_utility_hook(process_utility_hook);
}
Expand All @@ -251,11 +271,15 @@ void handle_drop_extension(void (*process_utility_hook)(PROCESS_UTILITY_PARAMS),
}

if (all_extensions_are_privileged) {
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser,
&already_switched_to_superuser);

run_process_utility_hook(process_utility_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}
} else {
run_process_utility_hook(process_utility_hook);
}
Expand Down
43 changes: 31 additions & 12 deletions src/supautils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,14 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
}

{
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand Down Expand Up @@ -595,6 +598,7 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
case T_CreateFdwStmt:
{
const Oid current_user_id = GetUserId();
bool already_switched_to_superuser = false;

if (superuser()) {
break;
Expand All @@ -603,7 +607,7 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
break;
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

Expand Down Expand Up @@ -646,7 +650,9 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
PopActiveSnapshot();
}

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand All @@ -657,6 +663,7 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
case T_CreatePublicationStmt:
{
const Oid current_user_id = GetUserId();
bool already_switched_to_superuser = false;

if (superuser()) {
break;
Expand All @@ -665,7 +672,7 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
break;
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

Expand Down Expand Up @@ -703,7 +710,9 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
PopActiveSnapshot();
}

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand All @@ -713,18 +722,22 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
*/
case T_AlterPublicationStmt:
{
bool already_switched_to_superuser = false;

if (superuser()) {
break;
}
if (!is_privileged_role()) {
break;
}

switch_to_superuser(privileged_extensions_superuser);
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand Down Expand Up @@ -770,11 +783,14 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
}

{
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand Down Expand Up @@ -802,11 +818,14 @@ supautils_hook(PROCESS_UTILITY_PARAMS)
}

{
switch_to_superuser(privileged_extensions_superuser);
bool already_switched_to_superuser = false;
switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser);

run_process_utility_hook(prev_hook);

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

return;
}
Expand Down
20 changes: 17 additions & 3 deletions src/utils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@
static Oid prev_role_oid = 0;
static int prev_role_sec_context = 0;

// Prevent nested switch_to_superuser() calls from corrupting prev_role_*
static bool is_switched_to_superuser = false;

static bool strstarts(const char *, const char *);

void alter_role_with_bypassrls_option_as_superuser(const char *role_name,
DefElem *bypassrls_option,
const char *superuser_name) {
RoleSpec *role = makeNode(RoleSpec);
AlterRoleStmt *bypassrls_stmt = makeNode(AlterRoleStmt);
bool already_switched_to_superuser = false;

role->roletype = ROLESPEC_CSTRING;
role->rolename = pstrdup(role_name);
Expand All @@ -22,15 +26,17 @@ void alter_role_with_bypassrls_option_as_superuser(const char *role_name,
bypassrls_stmt->role = role;
bypassrls_stmt->options = list_make1(bypassrls_option);

switch_to_superuser(superuser_name);
switch_to_superuser(superuser_name, &already_switched_to_superuser);

#if PG15_GTE
AlterRole(NULL, bypassrls_stmt);
#else
AlterRole(bypassrls_stmt);
#endif

switch_to_original_role();
if (!already_switched_to_superuser) {
switch_to_original_role();
}

pfree(role->rolename);
pfree(role);
Expand All @@ -40,8 +46,15 @@ void alter_role_with_bypassrls_option_as_superuser(const char *role_name,
return;
}

void switch_to_superuser(const char *privileged_extensions_superuser) {
void switch_to_superuser(const char *privileged_extensions_superuser,
bool *already_switched) {
Oid superuser_oid = BOOTSTRAP_SUPERUSERID;
*already_switched = is_switched_to_superuser;

if (*already_switched) {
return;
}
is_switched_to_superuser = true;

if (privileged_extensions_superuser != NULL) {
superuser_oid = get_role_oid(privileged_extensions_superuser, false);
Expand All @@ -55,6 +68,7 @@ void switch_to_superuser(const char *privileged_extensions_superuser) {

void switch_to_original_role(void) {
SetUserIdAndSecContext(prev_role_oid, prev_role_sec_context);
is_switched_to_superuser = false;
}

bool is_string_in_comma_delimited_string(const char *s1, const char *s2) {
Expand Down
3 changes: 2 additions & 1 deletion src/utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ alter_role_with_bypassrls_option_as_superuser(const char *role_name,
* Switch to a superuser and save the original role. Caller is responsible for
* calling switch_to_original_role() afterwards.
*/
extern void switch_to_superuser(const char *privileged_extensions_superuser);
extern void switch_to_superuser(const char *privileged_extensions_superuser,
bool *already_switched);

/**
* Restore the saved original role. Caller is responsible for ensuring
Expand Down
Loading

0 comments on commit a335308

Please sign in to comment.