diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2022-07-19 17:22:03 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2022-07-19 17:22:31 -0400 |
commit | a2944d8724522c5659c024b191f2fbfa9770faaf (patch) | |
tree | a42857837a7d2885d116ff1767c6c35d87d7d9d5 /src/include/utils/guc_tables.h | |
parent | 795ccd44037cbe14a7366d90de94764a7136deb7 (diff) | |
download | postgresql-a2944d8724522c5659c024b191f2fbfa9770faaf.tar.gz postgresql-a2944d8724522c5659c024b191f2fbfa9770faaf.zip |
Fix missed corner cases for grantable permissions on GUCs.
We allow users to set the values of not-yet-loaded extension GUCs,
remembering those values in "placeholder" GUC entries. When/if
the extension is loaded later in the session, we need to verify that
the user had permissions to set the GUC. That was done correctly
before commit a0ffa885e, but as of that commit, we'd check the
permissions of the active role when the LOAD happens, not the role
that had set the value. (This'd be a security bug if it had made it
into a released version.)
In principle this is simple enough to fix: we just need to remember
the exact role OID that set each GUC value, and use that not
GetUserID() when verifying permissions. Maintaining that data in
the guc.c data structures is slightly tedious, but fortunately it's
all basically just copy-n-paste of the logic for tracking the
GucSource of each setting, as we were already doing.
Another oversight is that validate_option_array_item() hadn't
been taught to check for granted GUC privileges. This appears
to manifest only in that ALTER ROLE/DATABASE RESET ALL will
fail to reset settings that the user should be allowed to reset.
Patch by myself and Nathan Bossart, per report from Nathan Bossart.
Back-patch to v15 where the faulty code came in.
Discussion: https://postgr.es/m/20220706224727.GA2158260@nathanxps13
Diffstat (limited to 'src/include/utils/guc_tables.h')
-rw-r--r-- | src/include/utils/guc_tables.h | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h index ba44f7437bf..067d82ada95 100644 --- a/src/include/utils/guc_tables.h +++ b/src/include/utils/guc_tables.h @@ -120,6 +120,8 @@ typedef struct guc_stack /* masked value's source must be PGC_S_SESSION, so no need to store it */ GucContext scontext; /* context that set the prior value */ GucContext masked_scontext; /* context that set the masked value */ + Oid srole; /* role that set the prior value */ + Oid masked_srole; /* role that set the masked value */ config_var_value prior; /* previous value of variable */ config_var_value masked; /* SET value in a GUC_SET_LOCAL entry */ } GucStack; @@ -131,6 +133,10 @@ typedef struct guc_stack * applications may use the long description as well, and will append * it to the short description. (separated by a newline or '. ') * + * srole is the role that set the current value, or BOOTSTRAP_SUPERUSERID + * if the value came from an internal source or the config file. Similarly + * for reset_srole (which is usually BOOTSTRAP_SUPERUSERID, but not always). + * * Note that sourcefile/sourceline are kept here, and not pushed into stacked * values, although in principle they belong with some stacked value if the * active value is session- or transaction-local. This is to avoid bloating @@ -152,6 +158,8 @@ struct config_generic GucSource reset_source; /* source of the reset_value */ GucContext scontext; /* context that set the current value */ GucContext reset_scontext; /* context that set the reset value */ + Oid srole; /* role that set the current value */ + Oid reset_srole; /* role that set the reset value */ GucStack *stack; /* stacked prior values */ void *extra; /* "extra" pointer for current actual value */ char *last_reported; /* if variable is GUC_REPORT, value last sent |