On 10/06/10 00:24, Tom Lane wrote:
I wrote:
[ thinks for awhile... ] I wonder whether there is any way of locking
down pg_get_expr so that it throws an error if called with anything
except a suitable field from one of the system catalogs.
I did a bit of research into this idea. It looks at least somewhat
feasible:
* All PG versions back to 7.4 will pass the calling expression tree via
fcinfo->flinfo->fn_expr. Lack of that would be a showstopper, because
we can't change the FmgrInfo struct in back branches (ABI break). So we
can get the arguments and determine whether they are Vars.
* To determine which table a Var actually refers to, we must have access
to the rangetable, and in join cases we also need access to the plan
tree node containing the Var (since we have to drill down to the plan
node's inputs to resolve OUTER and INNER references). The rangetable is
reachable from the PlanState node, so it's sufficient to make that one
pointer available to functions. The obvious way to handle this is to
add a field to FmgrInfo, and I would suggest doing it that way as of
9.0. In the back branches, we could probably hack it without an ABI
break by having fn_expr point to some intermediate node type that
contains both the actual expression tree and the PlanState pointer
(probably, we'd make it point to FuncExprState instead of directly to
the FuncExpr, and then add the field to FuncExprState, which is far less
dangerous than redefining struct FmgrInfo). Now this depends on the
assumption that no external functions are doing anything with fn_expr
except passing it to the related fmgr.c routines, which we would teach
about this convention.
* Once we've got the above data it's a simple matter of adapting the
Var-interpretation logic used by EXPLAIN in order to find out the table
OID and column number, if any, represented by the Var. I'd suggest
adding such functions in fmgr.c to extend the API currently offered by
get_fn_expr_argtype() and friends. It seems possible that other
functions besides pg_get_expr might have use for such a capability.
What I'm suggesting is definitely not a trivial patch, and I would never
consider back-patching it if it weren't a security matter. But I think
it's doable and we'd be fixing the hole with a determinate amount of
work, whereas trying to verify the validity of pg_get_expr input
directly would be a never-ending source of more security bugs.
Yeah, seems like it would work.
You could avoid changing the meaning of fn_expr by putting the check in
the parse analysis phase, into transformFuncCall(). That would feel
safer at least for back-branches.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
