Hey, hackers, As part of building Postgres for a managed environment in Google Cloud SQL, we've made various small changes to permission checks to carefully limit what customers have access to.
We were making some changes in src/backend/commands/user.c and noticed some clunky logic related to verifying that the current user has sufficient permissions to perform an action: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/commands/user.c;h=0e6800bf3e4f3f584bbb0b2f6f8ae999f5d94bf1;hb=HEAD#l288 The checks for whether the current user can create a user with the SUPERUSER, REPLICATION, or BYPASSRLS attributes are chained together using if/else-if, before finally checking whether the user has CREATEROLE privileges in a final else. This construction is error-prone, since once one branch is visited, later ones are skipped, and it implicitly assumes that the permissions needed for each subsequent action are subsets of the permissions needed for the previous action. Since each branch requires SUPERUSER this is fine, but changing one of the checks could inadvertently allow users without the CREATEROLE permission to create users. A similar construction is used later in the file in the AlterRole function: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/commands/user.c;h=0e6800bf3e4f3f584bbb0b2f6f8ae999f5d94bf1;hb=HEAD#l717 This small patch simply modifies the code to replace the if/else-if chain with separate if statements, and always checks whether the user has the CREATEROLE privilege. (The have_createrole_privilege function includes another superuser check.) Given the current checks in each branch, this code is equivalent, but easier to modify in the future. - Paul
user-c-if-else-if.patch
Description: Binary data