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.

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 2cfa43caf9afaa9985ea574bd1dffbb2f9ef7f20..cd265b3ffa9ea271c9d7f4dc11d6b7772404c641 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,13 +8,16 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.4 2005/11/18 23:08:43 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/parser/parse_expr.c,v 1.163.2.5 2010/06/30 18:11:43 heikki 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"
@@ -31,7 +34,9 @@
 #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"
 
@@ -447,6 +452,90 @@ 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:

diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c
index 0a5016932c5b0f4ac9e3682a9a35112a790b2fc6..b7cd204ed2b52c377b77576558164168d97f7c07 100644 (file)
--- a/src/backend/tcop/fastpath.c
+++ b/src/backend/tcop/fastpath.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.69 2003/09/25 06:58:02 petere Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/tcop/fastpath.c,v 1.69.2.1 2010/06/30 18:11:43 heikki Exp $
  *
  * NOTES
  *       This cruft is the server side of PQfn.
@@ -27,6 +27,7 @@
 #include "mb/pg_wchar.h"
 #include "tcop/fastpath.h"
 #include "utils/acl.h"
+#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
@@ -339,6 +340,16 @@ HandleFunctionRequest(StringInfo msgBuf)
         */
        SetQuerySnapshot();
 
+       /*
+        * Restrict access to pg_get_expr(). This reflects the hack in
+        * transformExpr() in parse_expr.c, see comments there on FuncCall 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.
         */

diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 2834610ac687ded699d50fcafaee87c984693536..e9592ab38353cb71e8c15392f8d72f523f53ee1a 100644 (file)
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: pg_constraint.h,v 1.9 2003/08/08 21:42:32 momjian Exp $
+ * $Id: pg_constraint.h,v 1.9.4.1 2010/06/30 18:11:43 heikki Exp $
  *
  * NOTES
  *       the genbki.sh script reads this file and generates .bki
@@ -19,6 +19,8 @@
 #ifndef PG_CONSTRAINT_H
 #define PG_CONSTRAINT_H
 
+#include "nodes/pg_list.h"
+
 /* ----------------
  *             postgres.h contains the system type definitions and the
  *             CATALOG(), BOOTSTRAP and DATA() sugar words so this file