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-09 21:21:41
Message-ID: 20150709212141.GA12131@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Noah, Andres,

* 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.

Thanks!

Stephen

Attachment Content-Type Size
rls-copy.patch text/x-diff 10.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2015-07-09 21:32:40 Re: Further issues with jsonb semantics, documentation
Previous Message Pavel Stehule 2015-07-09 21:16:37 Re: PL/pgSQL, RAISE and error context