On 23/06/10 21:36, Robert Haas wrote:
On Mon, Jun 21, 2010 at 7:50 PM, Heikki Linnakangas
<heikki.linnakan...@enterprisedb.com>  wrote:
On 15/06/10 10:31, Heikki Linnakangas wrote:

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.

Here's a patch using that approach.

I grepped through PostgreSQL and pgadmin source code to find the system
columns where valid node-strings are stored:

pg_index.indexprs
pg_index.indprep
pg_attrdef.adbin
pg_proc.proargdefaults
pg_constraint.conbin

Am I missing anything?

I think that pg_type.typdefaultbin is used by pg_dump.

Yep, added that.

pg_rewrite.ev_qual, pg_rewrite.ev_action, pg_trigger.tgqual also
contain nodeToString() output but I didn't have any luck using them
with pg_get_expr() so maybe they don't need to be included.

I left them out.

The only other thing I notice is that, obviously, the FIXME comment
needs to be FIXMEd before commit.

Fixed.

I'd still be in favor of inserting at least some basic error checks
into readfuncs.c, though just in HEAD.  The restrictions implemented
here seem adequate to prevent a security vulnerability, but superusers
can still invoke those functions manually, and while superusers can
clearly crash the system in any number of ways, that doesn't seem (to
me) like an adequate justification for ignoring the return value of
strtok().  YMMV, of course.

Agreed. I'll do that as a separate patch.

Thanks for the review!

--
  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

Reply via email to