rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.

Completion of back-port of my patch of yesterday.
This commit is contained in:
Tom Lane 2010-07-30 17:57:32 +00:00
parent 678e0d92c2
commit 10c05657d3
4 changed files with 135 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
src/backend/parser/parse_expr.c
View File

@ -8,16 +8,13 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.5 2010/06/30 18:11:43 heikki Exp $
* $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.6 2010/07/30 17:57:32 tgl Exp $
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
#include "catalog/catname.h"
#include "catalog/pg_attrdef.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_operator.h"
#include "catalog/pg_proc.h"
#include "commands/dbcommands.h"
@ -34,9 +31,7 @@
#include "parser/parse_oper.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
#include "parser/parsetree.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
@ -452,90 +447,6 @@ transformExpr(ParseState *pstate, Node *expr)
fn->agg_star,
fn->agg_distinct,
false);
/*
* 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 = lfirst(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;
Index levelsup;
levelsup = var->varlevelsup;
while (levelsup-- > 0)
{
pstate = pstate->parentParseState;
Assert(pstate != NULL);
}
Assert(var->varno > 0 &&
(int) var->varno <= length(pstate->p_rtable));
rte = rt_fetch(var->varno, pstate->p_rtable);
if (rte->relid == get_system_catalog_relid(IndexRelationName))
{
if (var->varattno == Anum_pg_index_indexprs ||
var->varattno == Anum_pg_index_indpred)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(AttrDefaultRelationName))
{
if (var->varattno == Anum_pg_attrdef_adbin)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(ConstraintRelationName))
{
if (var->varattno == Anum_pg_constraint_conbin)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(TypeRelationName))
{
if (var->varattno == Anum_pg_type_typdefaultbin)
allowed = true;
}
}
if (!allowed)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("argument to pg_get_expr() must come from system catalogs")));
}
}
break;
}
case T_SubLink:

125
src/backend/parser/parse_func.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.161 2003/09/29 00:05:25 petere Exp $
* $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.161.2.1 2010/07/30 17:57:32 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -16,10 +16,14 @@
#include "access/heapam.h"
#include "catalog/catname.h"
#include "catalog/pg_attrdef.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_inherits.h"
#include "catalog/pg_proc.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "parser/parsetree.h"
#include "parser/parse_agg.h"
#include "parser/parse_coerce.h"
#include "parser/parse_expr.h"
@ -371,6 +375,9 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
errmsg("aggregates may not return sets")));
}
/* Hack to protect pg_get_expr() against misuse */
check_pg_get_expr_args(pstate, funcid, fargs);
return retval;
}
@ -1531,3 +1538,119 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError)
return LookupFuncName(funcname, argcount, argoids, noError);
}
/*
* Given an RT index and nesting depth, find the corresponding RTE.
* This is the inverse of RTERangeTablePosn.
*/
static RangeTblEntry *
GetRTEByRangeTablePosn(ParseState *pstate,
int varno,
int sublevels_up)
{
while (sublevels_up-- > 0)
{
pstate = pstate->parentParseState;
Assert(pstate != NULL);
}
Assert(varno > 0 && varno <= length(pstate->p_rtable));
return rt_fetch(varno, pstate->p_rtable);
}
/*
* 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(args != NIL);
arg = (Node *) lfirst(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 <= length(rte->joinaliasvars))
{
arg = (Node *) nth(var->varattno - 1, rte->joinaliasvars);
goto restart;
}
}
else if (rte->rtekind == RTE_RELATION)
{
if (rte->relid == get_system_catalog_relid(IndexRelationName))
{
if (var->varattno == Anum_pg_index_indexprs ||
var->varattno == Anum_pg_index_indpred)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(AttrDefaultRelationName))
{
if (var->varattno == Anum_pg_attrdef_adbin)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(ConstraintRelationName))
{
if (var->varattno == Anum_pg_constraint_conbin)
allowed = true;
}
else if (rte->relid == get_system_catalog_relid(TypeRelationName))
{
if (var->varattno == Anum_pg_type_typdefaultbin)
allowed = true;
}
}
}
if (!allowed)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("argument to pg_get_expr() must come from system catalogs")));
}

8
src/backend/parser/parse_oper.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.76 2003/10/06 20:09:47 tgl Exp $
* $Header: /cvsroot/pgsql/src/backend/parser/parse_oper.c,v 1.76.2.1 2010/07/30 17:57:32 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -932,6 +932,9 @@ make_scalar_array_op(ParseState *pstate, List *opname,
result->useOr = useOr;
result->args = args;
/* Hack to protect pg_get_expr() against misuse */
check_pg_get_expr_args(pstate, opform->oprcode, args);
ReleaseSysCache(tup);
return (Expr *) result;
@ -1005,5 +1008,8 @@ make_op_expr(ParseState *pstate, Operator op,
result->opretset = get_func_retset(opform->oprcode);
result->args = args;
/* Hack to protect pg_get_expr() against misuse */
check_pg_get_expr_args(pstate, opform->oprcode, args);
return (Expr *) result;
}

4
src/include/parser/parse_func.h
View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $Id: parse_func.h,v 1.50 2003/08/04 02:40:14 momjian Exp $
* $Id: parse_func.h,v 1.50.4.1 2010/07/30 17:57:32 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -78,4 +78,6 @@ extern Oid LookupFuncName(List *funcname, int nargs, const Oid *argtypes,
extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes,
bool noError);
extern void check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args);
#endif /* PARSE_FUNC_H */