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.

1 files changed, 1 insertions, 81 deletions

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index ff4d43caf0d..488235fbb59 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,16 +8,13 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.179.4.4 2010/06/30 18:11:31 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.179.4.5 2010/07/30 17:57:24 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"
@@ -35,7 +32,6 @@
 #include "parser/parse_relation.h"
 #include "parser/parse_type.h"
 #include "utils/builtins.h"
-#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
@@ -439,82 +435,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 = 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);
-
-							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: