Skip to content

Commit 3613d08

Browse files
tglsfdctglsfdc
committed
Further fixes to the pg_get_expr() security fix in back branches.
It now emerges that the JDBC driver expects to be able to use pg_get_expr() on an output of a sub-SELECT. So extend the check logic to be able to recurse into a sub-SELECT to see if the argument is ultimately coming from an appropriate column. Per report from Thomas Kellerer.
1 parent f27860d commit 3613d08

File tree

1 file changed

+58
-26
lines changed

1 file changed

+58
-26
lines changed

‎src/backend/parser/parse_func.c

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include "parser/parse_relation.h"
2929
#include "parser/parse_target.h"
3030
#include "parser/parse_type.h"
31+
#include "parser/parsetree.h"
3132
#include "utils/builtins.h"
3233
#include "utils/fmgroids.h"
3334
#include "utils/lsyscache.h"
@@ -37,6 +38,7 @@
3738
static Oid FuncNameAsType(List *funcname);
3839
static Node *ParseComplexProjection(ParseState *pstate, char *funcname,
3940
Node *first_arg, int location);
41+
static bool check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup);
4042

4143

4244
/*
@@ -1627,9 +1629,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
16271629
void
16281630
check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
16291631
{
1630-
bool allowed = false;
16311632
Node *arg;
1632-
int netlevelsup;
16331633

16341634
/* if not being called for pg_get_expr, do nothing */
16351635
if (fnoid != F_PG_GET_EXPR && fnoid != F_PG_GET_EXPR_EXT)
@@ -1641,66 +1641,98 @@ check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
16411641

16421642
/*
16431643
* The first argument must be a Var referencing one of the allowed
1644-
* system-catalog columns. It could be a join alias Var, though.
1644+
* system-catalog columns. It could be a join alias Var or subquery
1645+
* reference Var, though, so we need a recursive subroutine to chase
1646+
* through those possibilities.
16451647
*/
16461648
Assert(list_length(args) > 1);
16471649
arg = (Node *) linitial(args);
1648-
netlevelsup = 0;
16491650

1650-
restart:
1651-
if (IsA(arg, Var))
1651+
if (!check_pg_get_expr_arg(pstate, arg, 0))
1652+
ereport(ERROR,
1653+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1654+
errmsg("argument to pg_get_expr() must come from system catalogs")));
1655+
}
1656+
1657+
static bool
1658+
check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup)
1659+
{
1660+
if (arg && IsA(arg, Var))
16521661
{
16531662
Var *var = (Var *) arg;
16541663
RangeTblEntry *rte;
1664+
AttrNumber attnum;
16551665

16561666
netlevelsup += var->varlevelsup;
16571667
rte = GetRTEByRangeTablePosn(pstate, var->varno, netlevelsup);
1668+
attnum = var->varattno;
16581669

16591670
if (rte->rtekind == RTE_JOIN)
16601671
{
1661-
/* Expand join alias reference */
1662-
if (var->varattno > 0 &&
1663-
var->varattno <= list_length(rte->joinaliasvars))
1672+
/* Recursively examine join alias variable */
1673+
if (attnum > 0 &&
1674+
attnum <= list_length(rte->joinaliasvars))
16641675
{
1665-
arg = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
1666-
goto restart;
1676+
arg = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
1677+
return check_pg_get_expr_arg(pstate, arg, netlevelsup);
16671678
}
16681679
}
1680+
else if (rte->rtekind == RTE_SUBQUERY)
1681+
{
1682+
/* Subselect-in-FROM: examine sub-select's output expr */
1683+
TargetEntry *ste = get_tle_by_resno(rte->subquery->targetList,
1684+
attnum);
1685+
ParseState mypstate;
1686+
1687+
if (ste == NULL || ste->resjunk)
1688+
elog(ERROR, "subquery %s does not have attribute %d",
1689+
rte->eref->aliasname, attnum);
1690+
arg = (Node *) ste->expr;
1691+
1692+
/*
1693+
* Recurse into the sub-select to see what its expr refers to.
1694+
* We have to build an additional level of ParseState to keep in
1695+
* step with varlevelsup in the subselect.
1696+
*/
1697+
MemSet(&mypstate, 0, sizeof(mypstate));
1698+
mypstate.parentParseState = pstate;
1699+
mypstate.p_rtable = rte->subquery->rtable;
1700+
/* don't bother filling the rest of the fake pstate */
1701+
1702+
return check_pg_get_expr_arg(&mypstate, arg, 0);
1703+
}
16691704
else if (rte->rtekind == RTE_RELATION)
16701705
{
16711706
switch (rte->relid)
16721707
{
16731708
case IndexRelationId:
1674-
if (var->varattno == Anum_pg_index_indexprs ||
1675-
var->varattno == Anum_pg_index_indpred)
1676-
allowed = true;
1709+
if (attnum == Anum_pg_index_indexprs ||
1710+
attnum == Anum_pg_index_indpred)
1711+
return true;
16771712
break;
16781713

16791714
case AttrDefaultRelationId:
1680-
if (var->varattno == Anum_pg_attrdef_adbin)
1681-
allowed = true;
1715+
if (attnum == Anum_pg_attrdef_adbin)
1716+
return true;
16821717
break;
16831718

16841719
case ProcedureRelationId:
1685-
if (var->varattno == Anum_pg_proc_proargdefaults)
1686-
allowed = true;
1720+
if (attnum == Anum_pg_proc_proargdefaults)
1721+
return true;
16871722
break;
16881723

16891724
case ConstraintRelationId:
1690-
if (var->varattno == Anum_pg_constraint_conbin)
1691-
allowed = true;
1725+
if (attnum == Anum_pg_constraint_conbin)
1726+
return true;
16921727
break;
16931728

16941729
case TypeRelationId:
1695-
if (var->varattno == Anum_pg_type_typdefaultbin)
1696-
allowed = true;
1730+
if (attnum == Anum_pg_type_typdefaultbin)
1731+
return true;
16971732
break;
16981733
}
16991734
}
17001735
}
17011736

1702-
if (!allowed)
1703-
ereport(ERROR,
1704-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1705-
errmsg("argument to pg_get_expr() must come from system catalogs")));
1737+
return false;
17061738
}

0 commit comments

Comments
 (0)