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

2021年1月28日(木) 17:18 Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>:

> On 2021-01-12 06:53, Ian Lawrence Barwick wrote:
> > 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.
>
> I'm not convinced the current behavior is wrong. Is there some
> practical use case that is affected by this behavior?
>

I was poking around at the function with a view to using it for something
and was
curious what it did with bad input.

Providing the column name of a dropped column:

Me: "Hey Postgres, do I have privileges on the dropped column 'bar' of
my table 'foo'?"
Pg: "That column doesn't even exist - here, have an error instead."
Me: "Hey Postgres, does some other less-privileged user have privileges
on the
dropped column 'bar' of my table 'foo'?
Pg: "That column doesn't even exist - here, have an error instead."

Providing the attnum of a dropped column:

Me: "Hey Postgres, here's the attnum of the dropped column 'bar', does
some
other less-privileged user have privileges on that column?"
Pg: "That column doesn't even exist - here, have a NULL".
Me: "Hey Postgres, here's the attnum of the dropped column 'bar' on
this table
I own, do I have privileges on that column?"
Pg: "Yup. And feel free to throw any other smallint at me, I'll pretend
that
represents a column too even if it never existed.".

Looking at the code, particularly the cited comment, it does seems the
intent was
to return NULL in all cases where an invalid attnum was provided, but that
gets
short-circuited by the assumption table owner = has privilege on any column.

Not the most urgent or exciting of issues, but seems inconsistent to me.

> > The second patch adds a bunch of missing static prototypes to "acl.c",
> > on general
> > principles.
>
> Why is this necessary?
>

Not exactly necessary, but seemed odd some functions had prototypes, others
not.
I have no idea what the policy is, if any, and not something I would lose
sleep over,
just thought I'd note it in passing.

Regards

Ian Barwick

--
EnterpriseDB: https://www.enterprisedb.com