164 lines
6.6 KiB
Diff
164 lines
6.6 KiB
Diff
From eeede2470a8ec902c80de449d2c4822330c689ca Mon Sep 17 00:00:00 2001
|
|
From: wang_yue111 <648774160@qq.com>
|
|
Date: Fri, 26 Feb 2021 12:57:48 +0800
|
|
Subject: [PATCH] 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
|
|
|
|
---
|
|
src/backend/parser/parse_relation.c | 40 ++++++++++-----------
|
|
src/backend/parser/parse_target.c | 8 +++--
|
|
4 files changed, 91 insertions(+), 22 deletions(-)
|
|
|
|
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
|
|
index bbbb0b6..7ab4ba7 100644
|
|
--- a/src/backend/parser/parse_relation.c
|
|
+++ b/src/backend/parser/parse_relation.c
|
|
@@ -43,7 +43,8 @@ static RangeTblEntry *scanNameSpaceForRelid(ParseState *pstate, Oid relid,
|
|
int location);
|
|
static void check_lateral_ref_ok(ParseState *pstate, ParseNamespaceItem *nsitem,
|
|
int location);
|
|
-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,
|
|
@@ -897,20 +898,15 @@ searchRangeTableForCol(ParseState *pstate, const char *alias, char *colname,
|
|
|
|
/*
|
|
* 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
|
|
- *
|
|
- * The caller should pass the actual RTE if it has it handy; otherwise pass
|
|
- * NULL, and we'll look it up here. (This uglification of the API is
|
|
- * worthwhile because nearly all external callers have the RTE at hand.)
|
|
*/
|
|
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)
|
|
{
|
|
@@ -942,13 +938,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",
|
|
@@ -957,13 +953,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",
|
|
@@ -994,10 +990,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 caller should pass the Var's referenced RTE if it has it handy
|
|
- * (nearly all do); otherwise pass NULL.
|
|
+ * The rte argument is unused and will be removed later.
|
|
*/
|
|
void
|
|
markVarForSelectPriv(ParseState *pstate, Var *var, RangeTblEntry *rte)
|
|
@@ -1008,7 +1004,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);
|
|
}
|
|
|
|
/*
|
|
@@ -2629,9 +2625,13 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte,
|
|
/*
|
|
* 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 64a1b75..c7165cb 100644
|
|
--- a/src/backend/parser/parse_target.c
|
|
+++ b/src/backend/parser/parse_target.c
|
|
@@ -1328,9 +1328,13 @@ ExpandSingleTable(ParseState *pstate, RangeTblEntry *rte,
|
|
/*
|
|
* 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)
|