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

On Mon, Jun 21, 2010 at 7:50 PM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)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.
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.

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

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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company