Fix mishandling of column-level SELECT privileges for join aliases.

scanNSItemForColumn, expandNSItemAttrs, and ExpandSingleTable would
pass the wrong RTE to markVarForSelectPriv when dealing with a join
ParseNamespaceItem: they'd pass the join RTE, when what we need to
mark is the base table that the join column came from.  The end
result was to not fill the base table's selectedCols bitmap correctly,
resulting in an understatement of the set of columns that are read
by the query.  The executor would still insist on there being at
least one selectable column; but with a correctly crafted query,
a user having SELECT privilege on just one column of a table would
nonetheless be allowed to read all its columns.

To fix, make markRTEForSelectPriv fetch the correct RTE for itself,
ignoring the possibly-mismatched RTE passed by the caller.  Later,
we'll get rid of some now-unused RTE arguments, but that risks
API breaks so we won't do it in released branches.

This problem was introduced by commit 9ce77d75c, so back-patch
to v13 where that came in.  Thanks to Sven Klemm for reporting
the problem.

Security: CVE-2021-20229

diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 43db4e9af8bf685e036f878d43adc889c67bc120..d0ec95c77986ef747348eb903454c79df6f37ae9 100644 (file)
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -68,7 +68,7 @@ static int    scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
                             const char *colname, int location,
                             int fuzzy_rte_penalty,
                             FuzzyAttrMatchState *fuzzystate);
-static void markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
+static void markRTEForSelectPriv(ParseState *pstate,
                                 int rtindex, AttrNumber col);
 static void expandRelation(Oid relid, Alias *eref,
                           int rtindex, int sublevels_up,
@@ -660,8 +660,8 @@ updateFuzzyAttrMatchState(int fuzzy_rte_penalty,
  *   If found, return an appropriate Var node, else return NULL.
  *   If the name proves ambiguous within this nsitem, raise error.
  *
- * Side effect: if we find a match, mark the item's RTE as requiring read
- * access for the column.
+ * Side effect: if we find a match, mark the corresponding RTE as requiring
+ * read access for the column.
  */
 Node *
 scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
@@ -990,21 +990,15 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam
 
 /*
  * markRTEForSelectPriv
- *    Mark the specified column of an RTE as requiring SELECT privilege
+ *    Mark the specified column of the RTE with index rtindex
+ *    as requiring SELECT privilege
  *
  * col == InvalidAttrNumber means a "whole row" reference
- *
- * External callers should always pass the Var's RTE.  Internally, we
- * allow NULL to be passed for the RTE and then look it up if needed;
- * this takes less code than requiring each internal recursion site
- * to perform a lookup.
  */
 static void
-markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
-                    int rtindex, AttrNumber col)
+markRTEForSelectPriv(ParseState *pstate, int rtindex, AttrNumber col)
 {
-   if (rte == NULL)
-       rte = rt_fetch(rtindex, pstate->p_rtable);
+   RangeTblEntry *rte = rt_fetch(rtindex, pstate->p_rtable);
 
    if (rte->rtekind == RTE_RELATION)
    {
@@ -1036,13 +1030,13 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
            {
                int         varno = ((RangeTblRef *) j->larg)->rtindex;
 
-               markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
+               markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
            }
            else if (IsA(j->larg, JoinExpr))
            {
                int         varno = ((JoinExpr *) j->larg)->rtindex;
 
-               markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
+               markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
            }
            else
                elog(ERROR, "unrecognized node type: %d",
@@ -1051,13 +1045,13 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
            {
                int         varno = ((RangeTblRef *) j->rarg)->rtindex;
 
-               markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
+               markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
            }
            else if (IsA(j->rarg, JoinExpr))
            {
                int         varno = ((JoinExpr *) j->rarg)->rtindex;
 
-               markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
+               markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
            }
            else
                elog(ERROR, "unrecognized node type: %d",
@@ -1078,7 +1072,10 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
 
 /*
  * markVarForSelectPriv
- *    Mark the RTE referenced by a Var as requiring SELECT privilege
+ *    Mark the RTE referenced by the Var as requiring SELECT privilege
+ *    for the Var's column (the Var could be a whole-row Var, too)
+ *
+ * The rte argument is unused and will be removed later.
  */
 void
 markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
@@ -1089,7 +1086,7 @@ markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
    /* Find the appropriate pstate if it's an uplevel Var */
    for (lv = 0; lv < var->varlevelsup; lv++)
        pstate = pstate->parentParseState;
-   markRTEForSelectPriv(pstate, rte, var->varno, var->varattno);
+   markRTEForSelectPriv(pstate, var->varno, var->varattno);
 }
 
 /*
@@ -3105,9 +3102,13 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
    /*
     * Require read access to the table.  This is normally redundant with the
     * markVarForSelectPriv calls below, but not if the table has zero
-    * columns.
+    * columns.  We need not do anything if the nsitem is for a join: its
+    * component tables will have been marked ACL_SELECT when they were added
+    * to the rangetable.  (This step changes things only for the target
+    * relation of UPDATE/DELETE, which cannot be under a join.)
     */
-   rte->requiredPerms |= ACL_SELECT;
+   if (rte->rtekind == RTE_RELATION)
+       rte->requiredPerms |= ACL_SELECT;
 
    forboth(name, names, var, vars)
    {

diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 51ecc16c42efe4eb7d7b7e706755a688496e4bc3..f582665aeae659ec5039a48e74044349684c1b45 100644 (file)
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -1384,9 +1384,13 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
        /*
         * Require read access to the table.  This is normally redundant with
         * the markVarForSelectPriv calls below, but not if the table has zero
-        * columns.
+        * columns.  We need not do anything if the nsitem is for a join: its
+        * component tables will have been marked ACL_SELECT when they were
+        * added to the rangetable.  (This step changes things only for the
+        * target relation of UPDATE/DELETE, which cannot be under a join.)
         */
-       rte->requiredPerms |= ACL_SELECT;
+       if (rte->rtekind == RTE_RELATION)
+           rte->requiredPerms |= ACL_SELECT;
 
        /* Require read access to each column */
        foreach(l, vars)

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index c5bd4e75e33c9e051df29c347973acdf4df41072..46a69fc0dc9dc5c1a7d990d155ff74bb7e05a1cd 100644 (file)
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -476,8 +476,54 @@ SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
 ERROR:  permission denied for table atest5
 SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
 ERROR:  permission denied for table atest5
+SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
+SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
 SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
 ERROR:  permission denied for table atest5
+SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
+ one 
+-----
+   1
+(1 row)
+
+SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
+ one 
+-----
+   1
+(1 row)
+
+SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
+SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
+SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
+SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+ERROR:  permission denied for table atest5
+SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
+SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+ERROR:  permission denied for table atest5
 SELECT 1 FROM atest5 WHERE two = 2; -- fail
 ERROR:  permission denied for table atest5
 SELECT * FROM atest1, atest5; -- fail

diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index e437006e53f20f3c7ec40186f27c7f3eb84d0ff5..6277140cfd3e38b5d1696eaa6c91067279ec9caf 100644 (file)
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -303,7 +303,26 @@ SELECT 1 FROM atest5; -- ok
 SELECT 1 FROM atest5 a JOIN atest5 b USING (one); -- ok
 SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
 SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
+SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
+SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
 SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
+SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
+SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
+SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
+SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
+SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
+SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
 SELECT 1 FROM atest5 WHERE two = 2; -- fail
 SELECT * FROM atest1, atest5; -- fail
 SELECT atest1.* FROM atest1, atest5; -- ok