"has_column_privilege()" issue with attnums and non-existent columns

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

Responses

Browse pgsql-hackers by date

  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