Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c028faf

Browse filesBrowse files
tglsfdctglsfdc
committed
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 9ce77d7, so back-patch to v13 where that came in. Thanks to Sven Klemm for reporting the problem. Security: CVE-2021-20229
1 parent 6214e2b commit c028faf
Copy full SHA for c028faf

File tree

Expand file treeCollapse file tree

4 files changed

+93
-23
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+93
-23
lines changed

‎src/backend/parser/parse_relation.c

Copy file name to clipboardExpand all lines: src/backend/parser/parse_relation.c
+22-21Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ static int scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
6868
const char *colname, int location,
6969
int fuzzy_rte_penalty,
7070
FuzzyAttrMatchState *fuzzystate);
71-
static void markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
71+
static void markRTEForSelectPriv(ParseState *pstate,
7272
int rtindex, AttrNumber col);
7373
static void expandRelation(Oid relid, Alias *eref,
7474
int rtindex, int sublevels_up,
@@ -660,8 +660,8 @@ updateFuzzyAttrMatchState(int fuzzy_rte_penalty,
660660
* If found, return an appropriate Var node, else return NULL.
661661
* If the name proves ambiguous within this nsitem, raise error.
662662
*
663-
* Side effect: if we find a match, mark the item's RTE as requiring read
664-
* access for the column.
663+
* Side effect: if we find a match, mark the corresponding RTE as requiring
664+
* read access for the column.
665665
*/
666666
Node *
667667
scanNSItemForColumn(ParseState *pstate, ParseNamespaceItem *nsitem,
@@ -990,21 +990,15 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, const char *colnam
990990

991991
/*
992992
* markRTEForSelectPriv
993-
* Mark the specified column of an RTE as requiring SELECT privilege
993+
* Mark the specified column of the RTE with index rtindex
994+
* as requiring SELECT privilege
994995
*
995996
* col == InvalidAttrNumber means a "whole row" reference
996-
*
997-
* External callers should always pass the Var's RTE. Internally, we
998-
* allow NULL to be passed for the RTE and then look it up if needed;
999-
* this takes less code than requiring each internal recursion site
1000-
* to perform a lookup.
1001997
*/
1002998
static void
1003-
markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
1004-
int rtindex, AttrNumber col)
999+
markRTEForSelectPriv(ParseState *pstate, int rtindex, AttrNumber col)
10051000
{
1006-
if (rte == NULL)
1007-
rte = rt_fetch(rtindex, pstate->p_rtable);
1001+
RangeTblEntry *rte = rt_fetch(rtindex, pstate->p_rtable);
10081002

10091003
if (rte->rtekind == RTE_RELATION)
10101004
{
@@ -1036,13 +1030,13 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
10361030
{
10371031
int varno = ((RangeTblRef *) j->larg)->rtindex;
10381032

1039-
markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
1033+
markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
10401034
}
10411035
else if (IsA(j->larg, JoinExpr))
10421036
{
10431037
int varno = ((JoinExpr *) j->larg)->rtindex;
10441038

1045-
markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
1039+
markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
10461040
}
10471041
else
10481042
elog(ERROR, "unrecognized node type: %d",
@@ -1051,13 +1045,13 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
10511045
{
10521046
int varno = ((RangeTblRef *) j->rarg)->rtindex;
10531047

1054-
markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
1048+
markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
10551049
}
10561050
else if (IsA(j->rarg, JoinExpr))
10571051
{
10581052
int varno = ((JoinExpr *) j->rarg)->rtindex;
10591053

1060-
markRTEForSelectPriv(pstate, NULL, varno, InvalidAttrNumber);
1054+
markRTEForSelectPriv(pstate, varno, InvalidAttrNumber);
10611055
}
10621056
else
10631057
elog(ERROR, "unrecognized node type: %d",
@@ -1078,7 +1072,10 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
10781072

10791073
/*
10801074
* markVarForSelectPriv
1081-
* Mark the RTE referenced by a Var as requiring SELECT privilege
1075+
* Mark the RTE referenced by the Var as requiring SELECT privilege
1076+
* for the Var's column (the Var could be a whole-row Var, too)
1077+
*
1078+
* The rte argument is unused and will be removed later.
10821079
*/
10831080
void
10841081
markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
@@ -1089,7 +1086,7 @@ markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
10891086
/* Find the appropriate pstate if it's an uplevel Var */
10901087
for (lv = 0; lv < var->varlevelsup; lv++)
10911088
pstate = pstate->parentParseState;
1092-
markRTEForSelectPriv(pstate, rte, var->varno, var->varattno);
1089+
markRTEForSelectPriv(pstate, var->varno, var->varattno);
10931090
}
10941091

10951092
/*
@@ -3105,9 +3102,13 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
31053102
/*
31063103
* Require read access to the table. This is normally redundant with the
31073104
* markVarForSelectPriv calls below, but not if the table has zero
3108-
* columns.
3105+
* columns. We need not do anything if the nsitem is for a join: its
3106+
* component tables will have been marked ACL_SELECT when they were added
3107+
* to the rangetable. (This step changes things only for the target
3108+
* relation of UPDATE/DELETE, which cannot be under a join.)
31093109
*/
3110-
rte->requiredPerms |= ACL_SELECT;
3110+
if (rte->rtekind == RTE_RELATION)
3111+
rte->requiredPerms |= ACL_SELECT;
31113112

31123113
forboth(name, names, var, vars)
31133114
{

‎src/backend/parser/parse_target.c

Copy file name to clipboardExpand all lines: src/backend/parser/parse_target.c
+6-2Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1384,9 +1384,13 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
13841384
/*
13851385
* Require read access to the table. This is normally redundant with
13861386
* the markVarForSelectPriv calls below, but not if the table has zero
1387-
* columns.
1387+
* columns. We need not do anything if the nsitem is for a join: its
1388+
* component tables will have been marked ACL_SELECT when they were
1389+
* added to the rangetable. (This step changes things only for the
1390+
* target relation of UPDATE/DELETE, which cannot be under a join.)
13881391
*/
1389-
rte->requiredPerms |= ACL_SELECT;
1392+
if (rte->rtekind == RTE_RELATION)
1393+
rte->requiredPerms |= ACL_SELECT;
13901394

13911395
/* Require read access to each column */
13921396
foreach(l, vars)

‎src/test/regress/expected/privileges.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/privileges.out
+46Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,8 +476,54 @@ SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
476476
ERROR: permission denied for table atest5
477477
SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
478478
ERROR: permission denied for table atest5
479+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
480+
ERROR: permission denied for table atest5
481+
SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
482+
ERROR: permission denied for table atest5
479483
SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
480484
ERROR: permission denied for table atest5
485+
SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
486+
one
487+
-----
488+
1
489+
(1 row)
490+
491+
SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
492+
one
493+
-----
494+
1
495+
(1 row)
496+
497+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
498+
ERROR: permission denied for table atest5
499+
SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
500+
ERROR: permission denied for table atest5
501+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
502+
ERROR: permission denied for table atest5
503+
SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
504+
ERROR: permission denied for table atest5
505+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
506+
ERROR: permission denied for table atest5
507+
SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
508+
ERROR: permission denied for table atest5
509+
SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
510+
ERROR: permission denied for table atest5
511+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
512+
ERROR: permission denied for table atest5
513+
SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
514+
ERROR: permission denied for table atest5
515+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
516+
ERROR: permission denied for table atest5
517+
SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
518+
ERROR: permission denied for table atest5
519+
SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
520+
ERROR: permission denied for table atest5
521+
SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
522+
ERROR: permission denied for table atest5
523+
SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
524+
ERROR: permission denied for table atest5
525+
SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
526+
ERROR: permission denied for table atest5
481527
SELECT 1 FROM atest5 WHERE two = 2; -- fail
482528
ERROR: permission denied for table atest5
483529
SELECT * FROM atest1, atest5; -- fail

‎src/test/regress/sql/privileges.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/privileges.sql
+19Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,26 @@ SELECT 1 FROM atest5; -- ok
303303
SELECT 1 FROM atest5 a JOIN atest5 b USING (one); -- ok
304304
SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
305305
SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
306+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
307+
SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
306308
SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
309+
SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
310+
SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
311+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
312+
SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
313+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
314+
SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
315+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
316+
SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
317+
SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
318+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
319+
SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
320+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
321+
SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
322+
SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
323+
SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
324+
SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
325+
SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
307326
SELECT 1 FROM atest5 WHERE two = 2; -- fail
308327
SELECT * FROM atest1, atest5; -- fail
309328
SELECT atest1.* FROM atest1, atest5; -- ok

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.