Re: copy.c handling for RLS is insecure

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: copy.c handling for RLS is insecure
Date: 2015-07-27 21:02:31
Message-ID: 20150727210230.GL3587@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

All,

* Stephen Frost (sfrost(at)snowman(dot)net) wrote:
> * Andres Freund (andres(at)anarazel(dot)de) wrote:
> > On 2015-07-09 01:28:28 -0400, Noah Misch wrote:
> > > > - Keep the OID check, shouldn't hurt to have it
> > >
> > > What benefit is left?
> >
> > A bit of defense in depth. We execute user defined code in COPY
> > (e.g. BEFORE triggers). That user defined code could very well replace
> > the relation. Now I think right now that'd happen late enough, so the
> > second lookup already happened. But a bit more robust defense against
> > that sounds good to me.
>
> Attached patch keeps the relation locked, fully qualifies it when
> building up the query, and uses list_member_oid() to check that the
> relation's OID ends up in the resulting relationOids list (to address
> Noah's point that the planner doesn't guarantee the ordering; I doubt
> that list will ever be more than a few entries long).
>
> Also removes the misguided Assert().
>
> Barring objections, I'll commit this (and backpatch to 9.5) tomorrow.

Apologies for not pushing this before I left on vacation. I've done so
now.

Thanks!

Stephen

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2015-07-27 21:10:14 Re: RFC: replace pg_stat_activity.waiting with something more descriptive
Previous Message Stephen Frost 2015-07-27 20:58:19 Re: A little RLS oversight?