Re: copy.c handling for RLS is insecure - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: copy.c handling for RLS is insecure
Date
Msg-id 20141202163227.GS3342@tamriel.snowman.net
Whole thread Raw
In response to Re: copy.c handling for RLS is insecure  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: copy.c handling for RLS is insecure
List pgsql-hackers
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Thu, Nov 27, 2014 at 2:03 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > Alright, I've done the change to use the RangeVar from CopyStmt, but
> > also added a check wherein we verify that the relation's OID returned
> > from the planned query is the same as the relation's OID that we did the
> > RLS check on- if they're different, we throw an error.  Please let me
> > know if there are any remaining concerns.
>
> That's clearly an improvement, but I'm not sure it's water-tight.
> What if the name that originally referenced a table ended up
> referencing a view?  Then you could get
> list_length(plan->relationOids) != 1.

I'll test it out and see what happens.  Certainly a good question and
if there's an issue there then I'll get it addressed.

> (And, in that case, I also wonder if you could get
> eval_const_expressions() to do evil things on your behalf while
> planning.)

If it can be made to reference a view then there's an issue as the view
might include a function call itself which is provided by the attacker..
I'm not sure that we have to really worry about anything more
complicated than that.

Clearly, if we found a relation originally then we need that same
relation with the same OID after the conversion to a query.
Thanks,
    Stephen

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: superuser() shortcuts
Next
From: Robert Haas
Date:
Subject: Re: superuser() shortcuts