Re: [BUGS] Server crash while trying to read expression using pg_get_expr()

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: [BUGS] Server crash while trying to read expression using pg_get_expr()
Дата
Msg-id 1521.1276118668@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: [BUGS] Server crash while trying to read expression using pg_get_expr()  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: [BUGS] Server crash while trying to read expression using pg_get_expr()  (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>)
Список pgsql-hackers
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.

Comments?
        regards, tom lane


В списке pgsql-hackers по дате отправления:

Предыдущее
От: Simon Riggs
Дата:
Сообщение: Re: hot_standby = on
Следующее
От: Tom Lane
Дата:
Сообщение: Re: Keepalive for max_standby_delay