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

Joe:
I don't seem to find attachment.

Maybe attach again ?

Thanks

On Sun, Mar 7, 2021 at 11:12 AM Joe Conway <mail(at)joeconway(dot)com> wrote:

> On 3/3/21 9:43 AM, Joe Conway wrote:
> > On 3/3/21 8:50 AM, David Steele wrote:
> >> On 1/29/21 4:56 AM, Joe Conway wrote:
> >>> On 1/29/21 12:13 AM, Ian Lawrence Barwick wrote:
> >>>> 2021年1月28日(木) 17:18 Peter Eisentraut:
> >>>> 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.
> >>>
> >>> Nicely illustrated :-)
> >>>
> >>>> Not the most urgent or exciting of issues, but seems inconsistent to
> me.
> >>>
> >>> +1
> >>
> >> Peter, did Ian's explanation answer your concerns?
> >>
> >> Joe (or Peter), any interest in reviewing/committing this patch?
> >
> > Sure, I'll take a look
>
> Based on Tom's commit here:
>
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=3d0f68dd3
>
> it appears to me that the intent is to return NULL.
>
> However I was not to happy with the way the submitted patch added an
> argument to
> column_privilege_check(). It seems to me that it is better to just reorder
> the
> checks in column_privilege_check() a bit, as in the attached.
>
> I wasn't entirely sure it was ok to split up the check for dropped
> attribute and
> pg_attribute_aclcheck like I did, but when I tested the race condition (by
> pausing at pg_attribute_aclcheck and dropping the column in another
> session)
> everything seemed to work fine.
>
> Any comments?
>
> Thanks,
>
> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>
>