In HEAD only, revert kluge solution for preventing misuse of pg_get_expr().
authorTom Lane <[email protected]>
Fri, 3 Sep 2010 01:26:52 +0000 (01:26 +0000)
committerTom Lane <[email protected]>
Fri, 3 Sep 2010 01:26:52 +0000 (01:26 +0000)
A data-type-based solution, which is much cleaner and more bulletproof,
will follow shortly.  It seemed best to make this a separate commit though.

src/backend/parser/parse_func.c
src/backend/parser/parse_oper.c
src/backend/tcop/fastpath.c
src/include/parser/parse_func.h

index 88d31a34202d7680f8f391f898b692dabe03ec69..9b869c14c820e19207d29cf19072e3c9e60e7dd7 100644 (file)
@@ -8,18 +8,15 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.226 2010/08/05 21:45:35 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.227 2010/09/03 01:26:52 tgl 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 "funcapi.h"
-#include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parse_agg.h"
@@ -29,7 +26,6 @@
 #include "parser/parse_target.h"
 #include "parser/parse_type.h"
 #include "utils/builtins.h"
-#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
@@ -511,9 +507,6 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
                retval = (Node *) wfunc;
        }
 
-       /* Hack to protect pg_get_expr() against misuse */
-       check_pg_get_expr_args(pstate, funcid, fargs);
-
        return retval;
 }
 
@@ -1600,107 +1593,3 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError)
 
        return oid;
 }
-
-
-/*
- * 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(list_length(args) > 1);
-       arg = (Node *) linitial(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 <= list_length(rte->joinaliasvars))
-                       {
-                               arg = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
-                               goto restart;
-                       }
-               }
-               else if (rte->rtekind == RTE_RELATION)
-               {
-                       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")));
-}
index d7316ef9b624b3a9ebc3a21a3ed392387b67201d..f72b692db521524367258f8cbdf0f249165278c3 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.114 2010/07/29 23:16:33 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_oper.c,v 1.115 2010/09/03 01:26:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -869,9 +869,6 @@ make_op(ParseState *pstate, List *opname, Node *ltree, Node *rtree,
 
        ReleaseSysCache(tup);
 
-       /* Hack to protect pg_get_expr() against misuse */
-       check_pg_get_expr_args(pstate, result->opfuncid, args);
-
        return (Expr *) result;
 }
 
@@ -1000,9 +997,6 @@ make_scalar_array_op(ParseState *pstate, List *opname,
 
        ReleaseSysCache(tup);
 
-       /* Hack to protect pg_get_expr() against misuse */
-       check_pg_get_expr_args(pstate, result->opfuncid, args);
-
        return (Expr *) result;
 }
 
index 5870081fe303bfc7f4a751614ad83be406ee8445..a5c5a309828dff4c43a3de539615a2550894a2d4 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.105 2010/07/06 19:18:57 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/tcop/fastpath.c,v 1.106 2010/09/03 01:26:52 tgl Exp $
  *
  * NOTES
  *       This cruft is the server side of PQfn.
@@ -29,7 +29,6 @@
 #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"
@@ -348,16 +347,6 @@ 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.
         */
index 8b2fd94bbaa90b0904de6570f7e03b9cbae21329..50e69537a3b3efaa224a46236a707170bc206d2f 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.69 2010/07/29 23:16:33 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.70 2010/09/03 01:26:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -82,6 +82,4 @@ extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes,
 extern Oid LookupAggNameTypeNames(List *aggname, List *argtypes,
                                           bool noError);
 
-extern void check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args);
-
 #endif   /* PARSE_FUNC_H */