On Thu, Jul 14, 2022 at 03:57:35PM -0700, Nathan Bossart wrote:
> However, ALTER ROLE RESET ALL will be blocked, while resetting only the
> individual GUC will go through.
>
> postgres=> ALTER ROLE other RESET ALL;
> ALTER ROLE
> postgres=> SELECT * FROM pg_db_role_setting;
> setdatabase | setrole | setconfig
> -------------+---------+-------------------------
> 0 | 16385 | {zero_damaged_pages=on}
> (1 row)
>
> postgres=> ALTER ROLE other RESET zero_damaged_pages;
> ALTER ROLE
> postgres=> SELECT * FROM pg_db_role_setting;
> setdatabase | setrole | setconfig
> -------------+---------+-----------
> (0 rows)
>
> I think this is because GUCArrayReset() is the only caller of
> validate_option_array_item() that sets skipIfNoPermissions to true. The
> others fall through to set_config_option(), which does a
> pg_parameter_aclcheck(). So, you are right.
Here's a small patch that seems to fix this case. However, I wonder if a
better way to fix this is to provide a way to stop set_config_option() from
throwing errors (e.g., setting elevel to -1). That way, we could remove
the manual permissions checks in favor of always using the real ones, which
might help prevent similar bugs in the future.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d43..fbc1014824 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -11689,7 +11689,8 @@ validate_option_array_item(const char *name, const char *value,
* We cannot do any meaningful check on the value, so only permissions
* are useful to check.
*/
- if (superuser())
+ if (superuser() ||
+ pg_parameter_aclcheck(gconf->name, GetUserId(), ACL_SET) == ACLCHECK_OK)
return true;
if (skipIfNoPermissions)
return false;
@@ -11703,6 +11704,8 @@ validate_option_array_item(const char *name, const char *value,
/* ok */ ;
else if (gconf->context == PGC_SUSET && superuser())
/* ok */ ;
+ else if (pg_parameter_aclcheck(gconf->name, GetUserId(), ACL_SET) == ACLCHECK_OK)
+ /* ok */ ;
else if (skipIfNoPermissions)
return false;
/* if a permissions error should be thrown, let set_config_option do it */
