From: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
---|---|
To: | "110876189(at)qq(dot)com" <110876189(at)qq(dot)com>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
Subject: | Re: BUG #15371: a user who not a member of pg_read_server_files role can create a new user into pg_read_server_files |
Date: | 2018-09-08 07:37:01 |
Message-ID: | CAKFQuwZtXU8QD6kZ4BBNgq0F1xrwq+mrStUkTRt+-W3vSgeuaw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
On Saturday, September 8, 2018, PG Bug reporting form <
noreply(at)postgresql(dot)org> wrote:
>
> see the code src/backend/commands/user.c, the check privillige code is :
> static void
> AddRoleMems(const char *rolename, Oid roleid,
> List *memberSpecs, List *memberIds,
> Oid grantorId, bool admin_opt)
> {
> else
> {
> if (!have_createrole_privilege() &&
> !is_admin_of_role(grantorId, roleid))
> ereport(ERROR,
> (errcode(ERRCODE_INSUFFICIENT_
> PRIVILEGE),
> errmsg("must have admin option on
> role \"%s\"",
> rolename)));
> }
>
> I think the line "if (!have_createrole_privilege() &&
> !is_admin_of_role(grantorId, roleid))" should been modifed to "if
> (!have_createrole_privilege() || !is_admin_of_role(grantorId, roleid))" .
>
Security code would ideally be written so not erroring requires an actual
matched condition while leaving the default to be an error. So, I think,
the && is ok, remove both negations, put a commented no-op there, and move
the error to an else block.
Whether that is sufficient for this bug I do not know, but it would be a
more secure format.
David J.
From | Date | Subject | |
---|---|---|---|
Next Message | David G. Johnston | 2018-09-08 07:41:45 | Re: BUG #15371: a user who not a member of pg_read_server_files role can create a new user into pg_read_server_files |
Previous Message | PG Bug reporting form | 2018-09-08 07:08:22 | BUG #15372: pg_stat_statements extension ignore stats_temp_directory setting and always write into pg_stat_tmp |