Further fixes to the pg_get_expr() security fix in back branches.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 25 Sep 2010 19:57:05 +0000 (15:57 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 25 Sep 2010 20:20:50 +0000 (16:20 -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 45a1c284dfebe05d805c073f50b13010cb68f1cd..272e1b79a16c7004b4880d672bd298fcee52ddb2 100644 (file)
@@ -30,6 +30,7 @@
 #include "parser/parse_relation.h"
 #include "parser/parse_target.h"
 #include "parser/parse_type.h"
+#include "parser/parsetree.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
@@ -40,6 +41,7 @@ static Node *ParseComplexProjection(ParseState *pstate, char *funcname,
                                           Node *first_arg, int location);
 static void unknown_attribute(ParseState *pstate, Node *relref, char *attname,
                                  int location);
+static bool check_pg_get_expr_arg(ParseState *pstate, Node *arg, int netlevelsup);
 
 
 /*
@@ -1316,9 +1318,7 @@ LookupAggNameTypeNames(List *aggname, 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)
@@ -1330,61 +1330,93 @@ 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->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)
                {
                        switch (rte->relid)
                        {
                                case IndexRelationId:
-                                       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;
                                        break;
 
                                case AttrDefaultRelationId:
-                                       if (var->varattno == Anum_pg_attrdef_adbin)
-                                               allowed = true;
+                                       if (attnum == Anum_pg_attrdef_adbin)
+                                               return true;
                                        break;
 
                                case ConstraintRelationId:
-                                       if (var->varattno == Anum_pg_constraint_conbin)
-                                               allowed = true;
+                                       if (attnum == Anum_pg_constraint_conbin)
+                                               return true;
                                        break;
 
                                case TypeRelationId:
-                                       if (var->varattno == Anum_pg_type_typdefaultbin)
-                                               allowed = true;
+                                       if (attnum == Anum_pg_type_typdefaultbin)
+                                               return true;
                                        break;
                        }
                }
        }
 
-       if (!allowed)
-               ereport(ERROR,
-                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-                                errmsg("argument to pg_get_expr() must come from system catalogs")));
+       return false;
 }