Re: BUG #4350: 'select' acess given to views containing "union all" even though user has no grants

Heikki Linnakangas wrote:
> Brendan O'Shea wrote:
>> There appears to be a bug in the way that permissions are determined for
>> views that contain "UNION ALL" in their definition.
>> There is a simple test case to reproduce the bug.
>>
>> 1) As a superuser create the following objects:
>>
>> CREATE ROLE test_perm LOGIN PASSWORD 'test_perm';
>>
>> CREATE OR REPLACE VIEW public.simple_select AS SELECT 1;
>> CREATE OR REPLACE VIEW public.union_all AS SELECT 1 UNION ALL SELECT 2;
>>
>>
>> 2) Now log in as the test_perm user and run the following SQL:
>>
>> select * from public.simple_select;
>> select * from public.union_all;
>>
>> The first SQL statement correctly produces an error, but the second
>> statement will return results with no error, it should instead generate a
>> permission error.
>
> Hmm, looks like pull_up_subqueries somehow loses the range table entry
> referring the original view. It's reproducible on PG version 8.2 and
> higher, 8.1 seems to work. I'll dig deeper tomorrow, unless someone else
> beats me to it.

Yep, pull_up_simple_union_all neglects to copy those range table
references that are not directly referenced in any of the UNION ALL
subqueries.

Attached is a patch for this. I have to go to sleep now, but will commit
this tomorrow unless someone comes up with a better idea. One thing that
I'm not totally happy about is that I'm using list_union, which uses
O(n^2) algorithm.

Thanks for the report!

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
Index: src/backend/optimizer/prep/prepjointree.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/optimizer/prep/prepjointree.c,v
retrieving revision 1.44
diff -c -r1.44 prepjointree.c
*** src/backend/optimizer/prep/prepjointree.c    4 Oct 2006 00:29:54 -0000    1.44
--- src/backend/optimizer/prep/prepjointree.c    11 Aug 2008 19:54:30 -0000
***************
*** 487,492 ****
--- 487,500 ----
      pull_up_union_leaf_queries(subquery->setOperations, root, varno, subquery);

      /*
+      * pull_up_union_leaf_queries copied those range table entries that were
+      * referenced from one of the UNION ALL subqueries, but we need to make
+      * sure that all non-referenced ones are copied as well. They are needed
+      * for permission checks during executor startup (ExecCheckRTPerms).
+      */
+     root->parse->rtable = list_union(root->parse->rtable, subquery->rtable);
+
+     /*
       * Mark the parent as an append relation.
       */
      rte->inh = true;