Further fixes to the pg_get_expr() security fix in back branches.
authorTom Lane <[email protected]>
Sat, 25 Sep 2010 19:57:05 +0000 (15:57 -0400)
committerTom Lane <[email protected]>
Sat, 25 Sep 2010 20:53:26 +0000 (16:53 -0400)
It now emerges that the JDBC driver expects to be able to use pg_get_expr()
on an output of a sub-SELECT.  So extend the check logic to be able to recurse
into a sub-SELECT to see if the argument is ultimately coming from an
appropriate column.  Per report from Thomas Kellerer.

src/backend/parser/parse_func.c

index b46350678654e814a221e280f21f19d671509deb..516424ebdb59ba12b82f9d3e7e900276f151dbe6 100644 (file)
@@ -29,6 +29,7 @@
 #include "parser/parse_func.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"
@@ -42,6 +43,7 @@ static Oid **argtype_inherit(int nargs, Oid *argtypes);
 static int     find_inheritors(Oid relid, Oid **supervec);
 static Oid **gen_cross_product(InhPaths *arginh, int nargs);
 static void unknown_attribute(ParseState *pstate, Node *relref, char *attname);
+static bool check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup);
 
 
 /*
@@ -1439,9 +1441,7 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError)
 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)
@@ -1453,59 +1453,91 @@ check_pg_get_expr_args(ParseState *pstate, Oid fnoid, List *args)
 
        /*
         * The first argument must be a Var referencing one of the allowed
-        * system-catalog columns.  It could be a join alias Var, though.
+        * system-catalog columns.  It could be a join alias Var or subquery
+        * reference Var, though, so we need a recursive subroutine to chase
+        * through those possibilities.
         */
        Assert(list_length(args) > 1);
        arg = (Node *) linitial(args);
-       netlevelsup = 0;
 
-restart:
-       if (IsA(arg, Var))
+       if (!check_pg_get_expr_arg(pstate, arg, 0))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                errmsg("argument to pg_get_expr() must come from system catalogs")));
+}
+
+static bool
+check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup)
+{
+       if (arg && IsA(arg, Var))
        {
                Var                *var = (Var *) arg;
                RangeTblEntry *rte;
+               AttrNumber      attnum;
 
                netlevelsup += var->varlevelsup;
                rte = GetRTEByRangeTablePosn(pstate, var->varno, netlevelsup);
+               attnum = var->varattno;
 
                if (rte->rtekind == RTE_JOIN)
                {
-                       /* Expand join alias reference */
-                       if (var->varattno > 0 &&
-                               var->varattno <= list_length(rte->joinaliasvars))
+                       /* Recursively examine join alias variable */
+                       if (attnum > 0 &&
+                               attnum <= list_length(rte->joinaliasvars))
                        {
-                               arg = (Node *) list_nth(rte->joinaliasvars, var->varattno - 1);
-                               goto restart;
+                               arg = (Node *) list_nth(rte->joinaliasvars, attnum - 1);
+                               return check_pg_get_expr_arg(pstate, arg, netlevelsup);
                        }
                }
+               else if (rte->rtekind == RTE_SUBQUERY)
+               {
+                       /* Subselect-in-FROM: examine sub-select's output expr */
+                       TargetEntry *ste = get_tle_by_resno(rte->subquery->targetList,
+                                                                                               attnum);
+                       ParseState      mypstate;
+
+                       if (ste == NULL || ste->resdom->resjunk)
+                               elog(ERROR, "subquery %s does not have attribute %d",
+                                        rte->eref->aliasname, attnum);
+                       arg = (Node *) ste->expr;
+
+                       /*
+                        * Recurse into the sub-select to see what its expr refers to.
+                        * We have to build an additional level of ParseState to keep in
+                        * step with varlevelsup in the subselect.
+                        */
+                       MemSet(&mypstate, 0, sizeof(mypstate));
+                       mypstate.parentParseState = pstate;
+                       mypstate.p_rtable = rte->subquery->rtable;
+                       /* don't bother filling the rest of the fake pstate */
+
+                       return check_pg_get_expr_arg(&mypstate, arg, 0);
+               }
                else if (rte->rtekind == RTE_RELATION)
                {
                        if (rte->relid == get_system_catalog_relid(IndexRelationName))
                        {
-                               if (var->varattno == Anum_pg_index_indexprs ||
-                                       var->varattno == Anum_pg_index_indpred)
-                                       allowed = true;
+                               if (attnum == Anum_pg_index_indexprs ||
+                                       attnum == Anum_pg_index_indpred)
+                                       return true;
                        }
                        else if (rte->relid == get_system_catalog_relid(AttrDefaultRelationName))
                        {
-                               if (var->varattno == Anum_pg_attrdef_adbin)
-                                       allowed = true;
+                               if (attnum == Anum_pg_attrdef_adbin)
+                                       return true;
                        }
                        else if (rte->relid == get_system_catalog_relid(ConstraintRelationName))
                        {
-                               if (var->varattno == Anum_pg_constraint_conbin)
-                                       allowed = true;
+                               if (attnum == Anum_pg_constraint_conbin)
+                                       return true;
                        }
                        else if (rte->relid == get_system_catalog_relid(TypeRelationName))
                        {
-                               if (var->varattno == Anum_pg_type_typdefaultbin)
-                                       allowed = true;
+                               if (attnum == Anum_pg_type_typdefaultbin)
+                                       return true;
                        }
                }
        }
 
-       if (!allowed)
-               ereport(ERROR,
-                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                                errmsg("argument to pg_get_expr() must come from system catalogs")));
+       return false;
 }