From: | Ian Lawrence Barwick <barwick(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | "has_column_privilege()" issue with attnums and non-existent columns |
Date: | 2021-01-12 05:53:41 |
Message-ID: | CAB8KJ=iqpgpVqMG9_nY_7htPiqnTBKiT7guCf38fZyexmg4Z+w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Greetings
Consider a table set up as follows:
CREATE TABLE foo (id INT, val TEXT);
ALTER TABLE foo DROP COLUMN val;
When specifying the name of a dropped or non-existent column, the function
"has_column_privilege()" works as expected:
postgres=# SELECT has_column_privilege('foo', 'val', 'SELECT');
ERROR: column "val" of relation "foo" does not exist
postgres=# SELECT has_column_privilege('foo', 'bar', 'SELECT');
ERROR: column "bar" of relation "foo" does not exist
However when specifying a dropped or non-existent attnum, it returns
"TRUE", which is unexpected:
postgres=# SELECT has_column_privilege('foo', 2::int2, 'SELECT');
has_column_privilege
----------------------
t
(1 row)
postgres=# SELECT has_column_privilege('foo', 999::int2, 'SELECT');
has_column_privilege
----------------------
t
(1 row)
The comment on the relevant code section in "src/backend/utils/adt/acl.c"
(related to "column_privilege_check()") indicate that NULL is the intended
return value in these cases:
Likewise, the variants that take an integer attnum
return NULL (rather than throwing an error) if there is no such
pg_attribute entry. All variants return NULL if an attisdropped
column is selected.
The unexpected "TRUE" value is a result of "column_privilege_check()" returning
TRUE if the user has table-level privileges. This returns a valid result with
the function variants where the column name is specified, as the calling
function will have already performed a check of the column through its call to
"convert_column_name()". However when the attnum is specified, the status of
the column never gets checked.
Attached patch resolves this by having "column_privilege_check()" callers
indicate whether they have checked the column. If this is the case, and if the
user as table-level privileges, "column_privilege_check()" can return as before
with no further lookups needed.
If the user has table-level privileges but the column was not checked (i.e
attnum provided), the column lookup is performed.
The regression tests did contain checks for dropped/non-existent attnums,
however one group was acting on a table where the user had no table-level
privilege, giving a fall-through to the column-level check; the other group
contained a check which was just plain wrong.
This issue appears to have been present since "has_column_privilege()" was
introduced in PostreSQL 8.4 (commit 7449427a1e6a099bc7e76164cb99a01d5e87237b),
so falls into the "obscure, extreme corner-case" category, OTOH seems worth
backpatching to supported versions.
The second patch adds a bunch of missing static prototypes to "acl.c",
on general
principles.
I'll add this to the next commitfest.
Regards
Ian Barwick
--
EnterpriseDB: https://www.enterprisedb.com
Attachment | Content-Type | Size |
---|---|---|
has_column_privilege-attnum-fix.v1.patch | text/x-patch | 7.7 KB |
acl-missing-prototypes.v1.patch | text/x-patch | 1.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2021-01-12 05:55:34 | Re: O(n^2) system calls in RemoveOldXlogFiles() |
Previous Message | japin | 2021-01-12 05:47:28 | Re: Logical Replication - behavior of ALTER PUBLICATION .. DROP TABLE and ALTER SUBSCRIPTION .. REFRESH PUBLICATION |