All, I have discovered a bug with the COPY command, specifically related to RLS.
The issue: When running COPY as superuser on a table that has RLS enabled, RLS is bypassed and therefore no issue exists. However, when performing a COPY as a non-privileged user on the same table causes issues when more than one column is specified as part of the command. Assuming: CREATE TABLE foo (a int, b int, c int); ... set up RLS Connecting as a non-privileged user provides the following results: COPY foo TO stdout; -- pass COPY foo (a) TO stdout; -- pass COPY foo (a, b, c) TO stdout; -- fail The error related to the failure is the following: ERROR: missing FROM-clause entry for table "b" LINE 1: copy foo (a, b, c) to stdout; I don't believe this to be a vulnerability simply because it doesn't 'leak' any data to a non-privileged user, it will just throw an error. As well, this is only an issue when more than one column is specified and '*' isn't implied. I have attached a 'test' file that can be used to observe this behavior. I have verified that this is an issue on REL9_5_STABLE, REL9_6_STABLE and master. Solution: The issue seems to be that the target list for the resulting SELECT statement is not being built correctly. I have attached a proposed patch to fix this issue. As well, I have added a few regression tests for this case. If deemed appropriate, then I'll add this to the currently open Commitfest. Please let me know if there are any questions. Thanks, Adam
test-copy-rls.sql
Description: application/sql
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 432b0ca..a3777d9 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -871,6 +871,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, uint64 *processed)
ColumnRef *cr;
ResTarget *target;
RangeVar *from;
+ List *targetList = NIL;
if (is_from)
ereport(ERROR,
@@ -878,21 +879,62 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, uint64 *processed)
errmsg("COPY FROM not supported with row-level security"),
errhint("Use INSERT statements instead.")));
- /* Build target list */
- cr = makeNode(ColumnRef);
-
+ /*
+ * Build target list
+ *
+ * If no columns are specified in the attribute list of the COPY
+ * command, then the target list is 'all' columns. Therefore, '*'
+ * should be used as the target list for the resulting SELECT
+ * statement.
+ *
+ * In the case that columns are specified in the attribute list,
+ * create a ColumnRef and ResTarget for each column and add them to
+ * the target list for the resulting SELECT statement.
+ */
if (!stmt->attlist)
+ {
+ cr = makeNode(ColumnRef);
cr->fields = list_make1(makeNode(A_Star));
+ cr->location = 1;
+
+ target = makeNode(ResTarget);
+ target->name = NULL;
+ target->indirection = NIL;
+ target->val = (Node *) cr;
+ target->location = 1;
+
+ targetList = list_make1(target);
+ }
else
- cr->fields = stmt->attlist;
+ {
+ ListCell *lc;
+ int location = 1;
+
+ foreach(lc, stmt->attlist)
+ {
+ /*
+ * Build the ColumnRef for each column. The ColumnRef
+ * 'fields' property is a String 'Value' node (see
+ * nodes/value.h) that correspond to the column name
+ * respectively.
+ */
+ cr = makeNode(ColumnRef);
+ cr->fields = list_make1(lfirst(lc));
+ cr->location = location;
+
+ /* Build the ResTarget and add the ColumnRef to it. */
+ target = makeNode(ResTarget);
+ target->name = NULL;
+ target->indirection = NIL;
+ target->val = (Node *) cr;
+ target->location = location;
- cr->location = 1;
+ /* Add each column to the SELECT statements target list */
+ targetList = lappend(targetList, target);
- target = makeNode(ResTarget);
- target->name = NULL;
- target->indirection = NIL;
- target->val = (Node *) cr;
- target->location = 1;
+ location += 1;
+ }
+ }
/*
* Build RangeVar for from clause, fully qualified based on the
@@ -903,7 +945,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt, uint64 *processed)
/* Build query */
select = makeNode(SelectStmt);
- select->targetList = list_make1(target);
+ select->targetList = targetList;
select->fromClause = list_make1(from);
query = (Node *) select;
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 5f6260a..13251c6 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -460,9 +460,57 @@ select * from check_con_tbl;
(2 rows)
+-- test with RLS enabled.
+CREATE USER regress_rls_copy_user;
+CREATE TABLE rls_t1 (a int, b int, c int);
+COPY rls_t1 (a, b, c) from stdin;
+CREATE POLICY p1 ON rls_t1 FOR SELECT USING (a % 2 = 0);
+ALTER TABLE rls_t1 ENABLE ROW LEVEL SECURITY;
+ALTER TABLE rls_t1 FORCE ROW LEVEL SECURITY;
+GRANT SELECT ON TABLE rls_t1 TO regress_rls_copy_user;
+-- all columns
+COPY rls_t1 TO stdout;
+1 1 1
+2 2 2
+3 3 3
+4 4 4
+COPY rls_t1 (a, b, c) TO stdout;
+1 1 1
+2 2 2
+3 3 3
+4 4 4
+-- subset of columns
+COPY rls_t1 (a) TO stdout;
+1
+2
+3
+4
+COPY rls_t1 (a, b) TO stdout;
+1 1
+2 2
+3 3
+4 4
+SET SESSION AUTHORIZATION regress_rls_copy_user;
+-- all columns
+COPY rls_t1 TO stdout;
+2 2 2
+4 4 4
+COPY rls_t1 (a, b, c) TO stdout;
+2 2 2
+4 4 4
+-- subset of columns
+COPY rls_t1 (a) TO stdout;
+2
+4
+COPY rls_t1 (a, b) TO stdout;
+2 2
+4 4
+RESET SESSION AUTHORIZATION;
DROP TABLE forcetest;
DROP TABLE vistest;
DROP FUNCTION truncate_in_subxact();
DROP TABLE x, y;
+DROP TABLE rls_t1 CASCADE;
+DROP USER regress_rls_copy_user;
DROP FUNCTION fn_x_before();
DROP FUNCTION fn_x_after();
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 39a9deb..1066083 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -327,9 +327,48 @@ copy check_con_tbl from stdin;
\.
select * from check_con_tbl;
+-- test with RLS enabled.
+CREATE USER regress_rls_copy_user;
+CREATE TABLE rls_t1 (a int, b int, c int);
+
+COPY rls_t1 (a, b, c) from stdin;
+1 1 1
+2 2 2
+3 3 3
+4 4 4
+\.
+
+CREATE POLICY p1 ON rls_t1 FOR SELECT USING (a % 2 = 0);
+ALTER TABLE rls_t1 ENABLE ROW LEVEL SECURITY;
+ALTER TABLE rls_t1 FORCE ROW LEVEL SECURITY;
+
+GRANT SELECT ON TABLE rls_t1 TO regress_rls_copy_user;
+
+-- all columns
+COPY rls_t1 TO stdout;
+COPY rls_t1 (a, b, c) TO stdout;
+
+-- subset of columns
+COPY rls_t1 (a) TO stdout;
+COPY rls_t1 (a, b) TO stdout;
+
+SET SESSION AUTHORIZATION regress_rls_copy_user;
+
+-- all columns
+COPY rls_t1 TO stdout;
+COPY rls_t1 (a, b, c) TO stdout;
+
+-- subset of columns
+COPY rls_t1 (a) TO stdout;
+COPY rls_t1 (a, b) TO stdout;
+
+RESET SESSION AUTHORIZATION;
+
DROP TABLE forcetest;
DROP TABLE vistest;
DROP FUNCTION truncate_in_subxact();
DROP TABLE x, y;
+DROP TABLE rls_t1 CASCADE;
+DROP USER regress_rls_copy_user;
DROP FUNCTION fn_x_before();
DROP FUNCTION fn_x_after();
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
