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 7045c3af5b560e17eb9d71d59539e92d1194bdcf..1e0afb3482a731b569457da39bfe355104d05fde 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,13 +8,15 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.185.2.2 2005/11/22 18:23:13 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.185.2.3 2010/06/30 18:11:19 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.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"
@@ -32,6 +34,7 @@
 #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"
 
@@ -776,6 +779,7 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
 {
        List       *targs;
        ListCell   *args;
+       Node       *result;
 
        /*
         * Transform the list of arguments.  We use a shallow list copy and then
@@ -791,12 +795,87 @@ transformFuncCall(ParseState *pstate, FuncCall *fn)
                                                                         (Node *) lfirst(args));
        }
 
-       return ParseFuncOrColumn(pstate,
+       result = ParseFuncOrColumn(pstate,
                                                         fn->funcname,
                                                         targs,
                                                         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);
+
+                               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 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 7d001e0f82444eb53220eef0f774e9fa55dac68b..472a567d0fa2da66249c0fbf4e11fd84cec404c0 100644 (file)
--- 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.83.2.2 2006/06/11 15:49:36 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.83.2.3 2010/06/30 18:11:19 heikki Exp $
  *
  * NOTES
  *       This cruft is the server side of PQfn.
@@ -28,6 +28,7 @@
 #include "tcop/fastpath.h"
 #include "tcop/tcopprot.h"
 #include "utils/acl.h"
+#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
@@ -343,6 +344,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.
         */

diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index fdfc006a7c0929b107f4f84f6b4b458765d32fd9..5a22433b08aacbd3614316f255102026716facaf 100644 (file)
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.18.2.1 2005/11/22 18:23:27 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.18.2.2 2010/06/30 18:11:19 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(), BKI_BOOTSTRAP and DATA() sugar words so this file