Обсуждение: Replacing the pg_get_expr security hack with a datatype solution

We agreed that we ought to do $SUBJECT in 9.1.  Right offhand the
outlines of a cleaner solution look pretty obvious:

* Create a datatype with the same internal representation as TEXT;
make its input and recv routines throw errors, while the output
routines just reuse textout/textsend.

* Provide an implicit cast to (but not from) text, so that we don't
break any queries doing textual functions on these columns.

* Change all system catalog columns holding expression trees to be
declared as this type.

* Modify the at-risk functions to take inputs of this type instead
of text.

The only thing that seems like it might need discussion is the name
to give the datatype.  My first instinct was pg_expr or pg_expression,
but there are some cases where this doesn't exactly fit.  In particular,
pg_rewrite.ev_action contains a whole Query, in fact a list of them.
We could go with something like pg_parse_tree, perhaps.  Or maybe
that's overthinking it.

Note that this approach will prevent even superusers from injecting
manually-created values into these columns and functions.  I don't see a
problem with that, and note that if a superuser really wants to do it,
he can create a cast that will let him.  I can't remember ever having
needed to do such a thing myself, so I think the debugging use-case
for it is nonexistent.

Comments, suggestions?
        regards, tom lane

On 21 August 2010 20:30, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * Change all system catalog columns holding expression trees to be
> declared as this type.

*snip*

> We could go with something like pg_parse_tree, perhaps.  Or maybe
> that's overthinking it.

How about pg_expr_tree?

--
Thom Brown
Registered Linux user: #516935

On Aug 21, 2010, at 3:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We agreed that we ought to do $SUBJECT in 9.1.

One argument against this is that it might cause the current fix to get less testing.

...Robert

Robert Haas <robertmhaas@gmail.com> writes:
> On Aug 21, 2010, at 3:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> We agreed that we ought to do $SUBJECT in 9.1.

> One argument against this is that it might cause the current fix to get less testing.

Less testing than what?
        regards, tom lane

On Aug 21, 2010, at 4:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Aug 21, 2010, at 3:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> We agreed that we ought to do $SUBJECT in 9.1.
>
>> One argument against this is that it might cause the current fix to get less testing.
>
> Less testing than what?

Is this a smart remark? Less testing than it otherwise would, obviously.

...Robert

On lör, 2010-08-21 at 15:30 -0400, Tom Lane wrote:
> The only thing that seems like it might need discussion is the name
> to give the datatype.  My first instinct was pg_expr or pg_expression,
> but there are some cases where this doesn't exactly fit.  In
> particular,
> pg_rewrite.ev_action contains a whole Query, in fact a list of them. 

Perhaps pg_node then.

Peter Eisentraut <peter_e@gmx.net> writes:
> On lör, 2010-08-21 at 15:30 -0400, Tom Lane wrote:
>> The only thing that seems like it might need discussion is the name
>> to give the datatype.  My first instinct was pg_expr or pg_expression,
>> but there are some cases where this doesn't exactly fit.  In
>> particular,
>> pg_rewrite.ev_action contains a whole Query, in fact a list of them. 

> Perhaps pg_node then.

pg_node sounds like there's just one.  Maybe pg_node_tree?
        regards, tom lane