Improved version of patch to protect pg_get_expr() against misuse:
look through join alias Vars to avoid breaking join queries, and move the test to someplace where it will catch more possible ways of calling a function. We still ought to throw away the whole thing in favor of a data-type-based solution, but that's not feasible in the back branches. This needs to be back-patched further than 9.0, but I don't have time to do so today. Committing now so that the fix gets into 9.0beta4.
This commit is contained in:
Tom Lane
parent
5b8bd0529e
commit
f223bb7a41
@ -8,16 +8,13 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.256 2010/07/06 19:18:57 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.257 2010/07/29 23:16:33 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
|
||||
#include "postgres.h"
|
||||
|
||||
#include "catalog/pg_attrdef.h"
|
||||
#include "catalog/pg_constraint.h"
|
||||
#include "catalog/pg_proc.h"
|
||||
#include "catalog/pg_type.h"
|
||||
#include "commands/dbcommands.h"
|
||||
#include "miscadmin.h"
|
||||
@ -33,7 +30,6 @@
|
||||
#include "parser/parse_target.h"
|
||||
#include "parser/parse_type.h"
|
||||
#include "utils/builtins.h"
|
||||
#include "utils/fmgroids.h"
|
||||
#include "utils/lsyscache.h"
|
||||
#include "utils/xml.h"
|
||||
|
||||
@ -1214,7 +1210,6 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
|
||||
{
|
||||
List *targs;
|
||||
ListCell *args;
|
||||
Node *result;
|
||||
|
||||
/* Transform the list of arguments ... */
|
||||
targs = NIL;
|
||||
@ -1225,97 +1220,16 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
|
||||
}
|
||||
|
||||
/* ... and hand off to ParseFuncOrColumn */
|
||||
result = ParseFuncOrColumn(pstate,
|
||||
fn->funcname,
|
||||
targs,
|
||||
fn->agg_order,
|
||||
fn->agg_star,
|
||||
fn->agg_distinct,
|
||||
fn->func_variadic,
|
||||
fn->over,
|
||||
false,
|
||||
fn->location);
|
||||
|
||||
/*
|
||||
* pg_get_expr() is a system function that exposes the expression
|
||||
* deparsing functionality in ruleutils.c to users. Very handy, but it was
|
||||
* later realized that the functions in ruleutils.c don't check the input
|
||||
* rigorously, assuming it to come from system catalogs and to therefore
|
||||
* be valid. That makes it easy for a user to crash the backend by passing
|
||||
* a maliciously crafted string representation of an expression to
|
||||
* pg_get_expr().
|
||||
*
|
||||
* There's a lot of code in ruleutils.c, so it's not feasible to add
|
||||
* water-proof input checking after the fact. Even if we did it once, it
|
||||
* would need to be taken into account in any future patches too.
|
||||
*
|
||||
* Instead, we restrict pg_rule_expr() to only allow input from system
|
||||
* catalogs instead. This is a hack, but it's the most robust and easiest
|
||||
* to backpatch way of plugging the vulnerability.
|
||||
*
|
||||
* This is transparent to the typical usage pattern of
|
||||
* "pg_get_expr(systemcolumn, ...)", but will break "pg_get_expr('foo',
|
||||
* ...)", even if 'foo' is a valid expression fetched earlier from a
|
||||
* system catalog. Hopefully there's isn't many clients doing that out
|
||||
* there.
|
||||
*/
|
||||
if (result && IsA(result, FuncExpr) &&!superuser())
|
||||
{
|
||||
FuncExpr *fe = (FuncExpr *) result;
|
||||
|
||||
if (fe->funcid == F_PG_GET_EXPR || fe->funcid == F_PG_GET_EXPR_EXT)
|
||||
{
|
||||
Expr *arg = linitial(fe->args);
|
||||
bool allowed = false;
|
||||
|
||||
/*
|
||||
* Check that the argument came directly from one of the allowed
|
||||
* system catalog columns
|
||||
*/
|
||||
if (IsA(arg, Var))
|
||||
{
|
||||
Var *var = (Var *) arg;
|
||||
RangeTblEntry *rte;
|
||||
|
||||
rte = GetRTEByRangeTablePosn(pstate,
|
||||
var->varno, var->varlevelsup);
|
||||
|
||||
switch (rte->relid)
|
||||
{
|
||||
case IndexRelationId:
|
||||
if (var->varattno == Anum_pg_index_indexprs ||
|
||||
var->varattno == Anum_pg_index_indpred)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case AttrDefaultRelationId:
|
||||
if (var->varattno == Anum_pg_attrdef_adbin)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case ProcedureRelationId:
|
||||
if (var->varattno == Anum_pg_proc_proargdefaults)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case ConstraintRelationId:
|
||||
if (var->varattno == Anum_pg_constraint_conbin)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case TypeRelationId:
|
||||
if (var->varattno == Anum_pg_type_typdefaultbin)
|
||||
allowed = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!allowed)
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
|
||||
errmsg("argument to pg_get_expr() must come from system catalogs")));
|
||||
}
|
||||
}
|
||||
return result;
|
||||
return ParseFuncOrColumn(pstate,
|
||||
fn->funcname,
|
||||
targs,
|
||||
fn->agg_order,
|
||||
fn->agg_star,
|
||||
fn->agg_distinct,
|
||||
fn->func_variadic,
|
||||
fn->over,
|
||||
false,
|
||||
fn->location);
|
||||
}
|
||||
|
||||
static Node *
|
||||
|
||||
@ -8,15 +8,18 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.224 2010/05/30 18:10:40 tgl Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.225 2010/07/29 23:16:33 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
#include "postgres.h"
|
||||
|
||||
#include "catalog/pg_attrdef.h"
|
||||
#include "catalog/pg_constraint.h"
|
||||
#include "catalog/pg_proc.h"
|
||||
#include "catalog/pg_type.h"
|
||||
#include "funcapi.h"
|
||||
#include "miscadmin.h"
|
||||
#include "nodes/makefuncs.h"
|
||||
#include "nodes/nodeFuncs.h"
|
||||
#include "parser/parse_agg.h"
|
||||
@ -26,6 +29,7 @@
|
||||
#include "parser/parse_target.h"
|
||||
#include "parser/parse_type.h"
|
||||
#include "utils/builtins.h"
|
||||
#include "utils/fmgroids.h"
|
||||
#include "utils/lsyscache.h"
|
||||
#include "utils/syscache.h"
|
||||
|
||||
@ -494,6 +498,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
|
||||
retval = (Node *) wfunc;
|
||||
}
|
||||
|
||||
/* Hack to protect pg_get_expr() against misuse */
|
||||
check_pg_get_expr_args(pstate, funcid, fargs);
|
||||
|
||||
return retval;
|
||||
}
|
||||
|
||||
@ -1580,3 +1587,107 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
|
||||
|
||||
return oid;
|
||||
}
|
||||
|
||||
|
||||
/*
|
||||
* pg_get_expr() is a system function that exposes the expression
|
||||
* deparsing functionality in ruleutils.c to users. Very handy, but it was
|
||||
* later realized that the functions in ruleutils.c don't check the input
|
||||
* rigorously, assuming it to come from system catalogs and to therefore
|
||||
* be valid. That makes it easy for a user to crash the backend by passing
|
||||
* a maliciously crafted string representation of an expression to
|
||||
* pg_get_expr().
|
||||
*
|
||||
* There's a lot of code in ruleutils.c, so it's not feasible to add
|
||||
* water-proof input checking after the fact. Even if we did it once, it
|
||||
* would need to be taken into account in any future patches too.
|
||||
*
|
||||
* Instead, we restrict pg_rule_expr() to only allow input from system
|
||||
* catalogs. This is a hack, but it's the most robust and easiest
|
||||
* to backpatch way of plugging the vulnerability.
|
||||
*
|
||||
* This is transparent to the typical usage pattern of
|
||||
* "pg_get_expr(systemcolumn, ...)", but will break "pg_get_expr('foo',
|
||||
* ...)", even if 'foo' is a valid expression fetched earlier from a
|
||||
* system catalog. Hopefully there aren't many clients doing that out there.
|
||||
*/
|
||||
void
|
||||
check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
|
||||
{
|
||||
bool allowed = false;
|
||||
Node *arg;
|
||||
int netlevelsup;
|
||||
|
||||
/* if not being called for pg_get_expr, do nothing */
|
||||
if (fnoid != F_PG_GET_EXPR && fnoid != F_PG_GET_EXPR_EXT)
|
||||
return;
|
||||
|
||||
/* superusers are allowed to call it anyway (dubious) */
|
||||
if (superuser())
|
||||
return;
|
||||
|
||||
/*
|
||||
* The first argument must be a Var referencing one of the allowed
|
||||
* system-catalog columns. It could be a join alias Var, though.
|
||||
*/
|
||||
Assert(list_length(args) > 1);
|
||||
arg = (Node *) linitial(args);
|
||||
netlevelsup = 0;
|
||||
|
||||
restart:
|
||||
if (IsA(arg, Var))
|
||||
{
|
||||
Var *var = (Var *) arg;
|
||||
RangeTblEntry *rte;
|
||||
|
||||
netlevelsup += var->varlevelsup;
|
||||
rte = GetRTEByRangeTablePosn(pstate, var->varno, netlevelsup);
|
||||
|
||||
if (rte->rtekind == RTE_JOIN)
|
||||
{
|
||||
/* Expand join alias reference */
|
||||
if (var->varattno > 0 &&
|
||||
var->varattno <= list_length(rte->joinaliasvars))
|
||||
{
|
||||
arg = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
|
||||
goto restart;
|
||||
}
|
||||
}
|
||||
else if (rte->rtekind == RTE_RELATION)
|
||||
{
|
||||
switch (rte->relid)
|
||||
{
|
||||
case IndexRelationId:
|
||||
if (var->varattno == Anum_pg_index_indexprs ||
|
||||
var->varattno == Anum_pg_index_indpred)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case AttrDefaultRelationId:
|
||||
if (var->varattno == Anum_pg_attrdef_adbin)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case ProcedureRelationId:
|
||||
if (var->varattno == Anum_pg_proc_proargdefaults)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case ConstraintRelationId:
|
||||
if (var->varattno == Anum_pg_constraint_conbin)
|
||||
allowed = true;
|
||||
break;
|
||||
|
||||
case TypeRelationId:
|
||||
if (var->varattno == Anum_pg_type_typdefaultbin)
|
||||
allowed = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!allowed)
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
|
||||
errmsg("argument to pg_get_expr() must come from system catalogs")));
|
||||
}
|
||||
|
||||
@ -8,7 +8,7 @@
|
||||
*
|
||||
*
|
||||
* IDENTIFICATION
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.113 2010/02/26 02:00:52 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.114 2010/07/29 23:16:33 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -869,6 +869,9 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
|
||||
|
||||
ReleaseSysCache(tup);
|
||||
|
||||
/* Hack to protect pg_get_expr() against misuse */
|
||||
check_pg_get_expr_args(pstate, result->opfuncid, args);
|
||||
|
||||
return (Expr *) result;
|
||||
}
|
||||
|
||||
@ -997,6 +1000,9 @@ make_scalar_array_op(ParseState *pstate, List *opname,
|
||||
|
||||
ReleaseSysCache(tup);
|
||||
|
||||
/* Hack to protect pg_get_expr() against misuse */
|
||||
check_pg_get_expr_args(pstate, result->opfuncid, args);
|
||||
|
||||
return (Expr *) result;
|
||||
}
|
||||
|
||||
|
||||
@ -7,7 +7,7 @@
|
||||
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
|
||||
* Portions Copyright (c) 1994, Regents of the University of California
|
||||
*
|
||||
* $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.68 2010/01/02 16:58:07 momjian Exp $
|
||||
* $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.69 2010/07/29 23:16:33 tgl Exp $
|
||||
*
|
||||
*-------------------------------------------------------------------------
|
||||
*/
|
||||
@ -82,4 +82,6 @@ extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes,
|
||||
extern Oid LookupAggNameTypeNames(List *aggname, List *argtypes,
|
||||
bool noError);
|
||||
|
||||
extern void check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args);
|
||||
|
||||
#endif /* PARSE_FUNC_H */
|
||||
|
||||
Loading…
Reference in New Issue
Block a user