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 d525fbc

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 8e56684 commit d525fbc
Copy full SHA for d525fbc

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
/*
@@ -3031,9 +3028,13 @@ expandNSItemAttrs(ParseState *pstate, ParseNamespaceItem *nsitem,
30313028
/*
30323029
* Require read access to the table. This is normally redundant with the
30333030
* markVarForSelectPriv calls below, but not if the table has zero
3034-
* columns.
3031+
* columns. We need not do anything if the nsitem is for a join: its
3032+
* component tables will have been marked ACL_SELECT when they were added
3033+
* to the rangetable. (This step changes things only for the target
3034+
* relation of UPDATE/DELETE, which cannot be under a join.)
30353035
*/
3036-
rte->requiredPerms |= ACL_SELECT;
3036+
if (rte->rtekind == RTE_RELATION)
3037+
rte->requiredPerms |= ACL_SELECT;
30373038

30383039
forboth(name, names, var, vars)
30393040
{

‎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
@@ -1356,9 +1356,13 @@ ExpandSingleTable(ParseState *pstate, ParseNamespaceItem *nsitem,
13561356
/*
13571357
* Require read access to the table. This is normally redundant with
13581358
* the markVarForSelectPriv calls below, but not if the table has zero
1359-
* columns.
1359+
* columns. We need not do anything if the nsitem is for a join: its
1360+
* component tables will have been marked ACL_SELECT when they were
1361+
* added to the rangetable. (This step changes things only for the
1362+
* target relation of UPDATE/DELETE, which cannot be under a join.)
13601363
*/
1361-
rte->requiredPerms |= ACL_SELECT;
1364+
if (rte->rtekind == RTE_RELATION)
1365+
rte->requiredPerms |= ACL_SELECT;
13621366

13631367
/* Require read access to each column */
13641368
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
@@ -474,8 +474,54 @@ SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
474474
ERROR: permission denied for table atest5
475475
SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
476476
ERROR: permission denied for table atest5
477+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
478+
ERROR: permission denied for table atest5
479+
SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
480+
ERROR: permission denied for table atest5
477481
SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
478482
ERROR: permission denied for table atest5
483+
SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
484+
one
485+
-----
486+
1
487+
(1 row)
488+
489+
SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
490+
one
491+
-----
492+
1
493+
(1 row)
494+
495+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
496+
ERROR: permission denied for table atest5
497+
SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
498+
ERROR: permission denied for table atest5
499+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
500+
ERROR: permission denied for table atest5
501+
SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
502+
ERROR: permission denied for table atest5
503+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
504+
ERROR: permission denied for table atest5
505+
SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
506+
ERROR: permission denied for table atest5
507+
SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
508+
ERROR: permission denied for table atest5
509+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
510+
ERROR: permission denied for table atest5
511+
SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
512+
ERROR: permission denied for table atest5
513+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
514+
ERROR: permission denied for table atest5
515+
SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
516+
ERROR: permission denied for table atest5
517+
SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
518+
ERROR: permission denied for table atest5
519+
SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
520+
ERROR: permission denied for table atest5
521+
SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
522+
ERROR: permission denied for table atest5
523+
SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
524+
ERROR: permission denied for table atest5
479525
SELECT 1 FROM atest5 WHERE two = 2; -- fail
480526
ERROR: permission denied for table atest5
481527
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
@@ -301,7 +301,26 @@ SELECT 1 FROM atest5; -- ok
301301
SELECT 1 FROM atest5 a JOIN atest5 b USING (one); -- ok
302302
SELECT 1 FROM atest5 a JOIN atest5 b USING (two); -- fail
303303
SELECT 1 FROM atest5 a NATURAL JOIN atest5 b; -- fail
304+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
305+
SELECT j.* FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
304306
SELECT (j.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)) j; -- fail
307+
SELECT one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
308+
SELECT j.one FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- ok
309+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
310+
SELECT j.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
311+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
312+
SELECT j.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)) j; -- fail
313+
SELECT * FROM (atest5 a JOIN atest5 b USING (one)); -- fail
314+
SELECT a.* FROM (atest5 a JOIN atest5 b USING (one)); -- fail
315+
SELECT (a.*) IS NULL FROM (atest5 a JOIN atest5 b USING (one)); -- fail
316+
SELECT two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
317+
SELECT a.two FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
318+
SELECT y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
319+
SELECT b.y FROM (atest5 a JOIN atest5 b(one,x,y,z) USING (one)); -- fail
320+
SELECT y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
321+
SELECT b.y FROM (atest5 a LEFT JOIN atest5 b(one,x,y,z) USING (one)); -- fail
322+
SELECT y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
323+
SELECT b.y FROM (atest5 a FULL JOIN atest5 b(one,x,y,z) USING (one)); -- fail
305324
SELECT 1 FROM atest5 WHERE two = 2; -- fail
306325
SELECT * FROM atest1, atest5; -- fail
307326
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.