From 350ab443beba3ce8a7ddf2090a3694de330f6bb3 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 30 Jun 2010 18:10:23 +0000
Subject: [PATCH] stringToNode() and deparse_expression_pretty() crash on
invalid input, but we have nevertheless exposed them to users via
pg_get_expr(). It would be too much maintenance effort to rigorously check
the input, so put a hack in place instead to restrict pg_get_expr() so that
the argument must come from one of the system catalog columns known to
contain valid expressions.
Per report from Rushabh Lathia. Backpatch to 7.4 which is the oldest
supported version at the moment.
---
src/backend/parser/parse_expr.c | 89 ++++++++++++++++++++++++++++++++-
src/backend/tcop/fastpath.c | 13 ++++-
2 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 12c93e15f41..3d960089c3f 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,13 +8,16 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.254 2010/02/26 02:00:52 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.255 2010/06/30 18:10:23 heikki 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"
@@ -30,6 +33,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/xml.h"
@@ -1210,6 +1214,7 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
{
List *targs;
ListCell *args;
+ Node *result;
/* Transform the list of arguments ... */
targs = NIL;
@@ -1220,7 +1225,7 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
}
/* ... and hand off to ParseFuncOrColumn */
- return ParseFuncOrColumn(pstate,
+ result = ParseFuncOrColumn(pstate,
fn->funcname,
targs,
fn->agg_order,
@@ -1230,6 +1235,86 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
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;
}
static Node *
diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c
index 75e4791546e..6d224478734 100644
--- a/src/backend/tcop/fastpath.c
+++ b/src/backend/tcop/fastpath.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.103 2010/02/14 18:42:15 rhaas Exp $
+ * $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.104 2010/06/30 18:10:23 heikki Exp $
*
* NOTES
* This cruft is the server side of PQfn.
@@ -29,6 +29,7 @@
#include "tcop/fastpath.h"
#include "tcop/tcopprot.h"
#include "utils/acl.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
@@ -347,6 +348,16 @@ HandleFunctionRequest(StringInfo msgBuf)
aclcheck_error(aclresult, ACL_KIND_PROC,
get_func_name(fid));
+ /*
+ * Restrict access to pg_get_expr(). This reflects the hack in
+ * transformFuncCall() in parse_expr.c, see comments there for an
+ * explanation.
+ */
+ if ((fid == F_PG_GET_EXPR || fid == F_PG_GET_EXPR_EXT) && !superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("argument to pg_get_expr() must come from system catalogs")));
+
/*
* Prepare function call info block and insert arguments.
*/
--
GitLab