8000 Further fixes to the pg_get_expr() security fix in back branches. · danielcode/postgres@3c2da80 · GitHub
[go: up one dir, main page]

Skip to content

Commit 3c2da80

Browse files
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 5efa144 commit 3c2da80

File tree

1 file changed

+56
-24
lines changed

1 file changed

+56
-24
lines changed

src/backend/parser/parse_func.c

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "parser/parse_relation.h"
3131
#include "parser/parse_target.h"
3232
#include "parser/parse_type.h"
33+
#include "parser/parsetree.h"
3334
#include "utils/builtins.h"
3435
#include "utils/fmgroids.h"
3536
#include "utils/lsyscache.h"
@@ -39,6 +40,7 @@
3940
static Node *ParseComplexProjection(ParseState *pstate, char *funcname,
4041
Node *first_arg);
4142
static void unknown_attribute(ParseState *pstate, Node *relref, char *attname);
43+
static bool check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup);
4244

4345

4446
/*
@@ -1265,9 +1267,7 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError)
12651267
void
12661268
check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
12671269
{
1268-
bool allowed = false;
12691270
Node *arg;
1270-
int netlevelsup;
12711271

12721272
/* if not being called for pg_get_expr, do nothing */
12731273
if (fnoid != F_PG_GET_EXPR && fnoid != F_PG_GET_EXPR_EXT)
@@ -1279,61 +1279,93 @@ check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
12791279

12801280
/*
12811281
* The first argument must be a Var referencing one of the allowed
1282-
* system-catalog columns. It could be a join alias Var, though.
1282+
* system-catalog columns. It could be a join alias Var or subquery
1283+
* reference Var, though, so we need a recursive subroutine to chase
1284+
* through those possibilities.
12831285
*/
12841286
Assert(list_length(args) > 1);
12851287
arg = (Node *) linitial(args);
1286-
netlevelsup = 0;
12871288

1288-
restart:
1289-
if (IsA(arg, Var))
1289+
if (!check_pg_get_expr_arg(pstate, arg, 0))
1290+
ereport(ERROR,
1291+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1292+
errmsg("argument to pg_get_expr() must come from system catalogs")));
1293+
}
1294+
1295+
static bool
1296+
check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup)
1297+
{
1298+
if (arg && IsA(arg, Var))
12901299
{
12911300
Var *var = (Var *) arg;
12921301
RangeTblEntry *rte;
1302+
AttrNumber attnum;
12931303

12941304
netlevelsup += var->varlevelsup;
12951305
rte = GetRTEByRangeTablePosn(pstate, var->varno, netlevelsup);
1306+
attnum = var->varattno;
12961307

12971308
if (rte->rtekind == RTE_JOIN)
12981309
{
1299-
/* Expand join alias reference */
1300-
if (var->varattno > 0 &&
1301-
var->varattno <= list_length(rte->joinaliasvars))
1310+
/* Recursively examine join alias variable */
1311+
if (attnum > 0 &&
1312+
attnum <= list_length(rte->joinaliasvars))
13021313
{
1303-
arg = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
1304-
goto restart;
1314+
arg = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
1315+
return check_pg_get_expr_arg(pstate, arg, netlevelsup);
13051316
}
13061317
}
1318+
else if (rte->rtekind == RTE_SUBQUERY)
1319+
{
1320+
/* Subselect-in-FROM: examine sub-select's output expr */
1321+
TargetEntry *ste = get_tle_by_resno(rte->subquery->targetList,
1322+
attnum);
1323+
ParseState mypstate;
1324+
1325+
if (ste == NULL || ste->resjunk)
1326+
elog(ERROR, "subquery %s does not have attribute %d",
1327+
rte->eref->aliasname, attnum);
1328+
arg = (Node *) ste->expr;
1329+
1330+
/*
1331+
* Recurse into the sub-select to see what its expr refers to.
1332+
* We have to build an additional level of ParseState to keep in
1333+
* step with varlevelsup in the subselect.
1334+
*/
1335+
MemSet(&mypstate, 0, sizeof(mypstate));
1336+
mypstate.parentParseState = pstate;
1337+
mypstate.p_rtable = rte->subquery->rtable;
1338+
/* don't bother filling the rest of the fake pstate */
1339+
1340+
return check_pg_get_expr_arg(&mypstate, arg, 0);
1341+
}
13071342
else if (rte->rtekind == RTE_RELATION)
13081343
{
13091344
switch (rte->relid)
13101345
{
13111346
case IndexRelationId:
1312-
if (var->varattno == Anum_pg_index_indexprs ||
1313-
var->varattno == Anum_pg_index_indpred)
1314-
allowed = true;
1347+
if (attnum == Anum_pg_index_indexprs ||
1348+
attnum == Anum_pg_index_indpred)
1349+
return true;
13151350
break;
13161351

13171352
case AttrDefaultRelationId:
1318-
if (var->varattno == Anum_pg_attrdef_adbin)
1319-
allowed = true;
1353+
if (attnum == Anum_pg_attrdef_adbin)
1354+
return true;
13201355
break;
13211356

13221357
case ConstraintRelationId:
1323-
if (var->varattno == Anum_pg_constraint_conbin)
1324-
allowed = true;
1358+
if (attnum == Anum_pg_constraint_conbin)
1359+
return true;
13251360
break;
13261361

13271362
case TypeRelationId:
1328-
if (var->varattno == Anum_pg_type_typdefaultbin)
1329-
allowed = true;
1363+
if (attnum == Anum_pg_type_typdefaultbin)
1364+
return true;
13301365
break;
13311366
}
13321367
}
13331368
}
13341369

1335-
if (!allowed)
1336-
ereport(ERROR,
1337-
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1338-
errmsg("argument to pg_get_expr() must come from system catalogs")));
1370+
return false;
13391371
}

0 commit comments

Comments
 (0)
0