Re: info about patch: using parametrised query in psql - Mailing list pgsql-hackers

From Robert Haas
Subject Re: info about patch: using parametrised query in psql
Date
Msg-id 603c8f070912241828g67ce2bf6uc0d72a7f3adec79e@mail.gmail.com
Whole thread Raw
In response to info about patch: using parametrised query in psql  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: info about patch: using parametrised query in psql  (Pavel Stehule <pavel.stehule@gmail.com>)
Re: info about patch: using parametrised query in psql  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Thu, Dec 24, 2009 at 2:45 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hello
>
> I try to explain my motivation for creating this patch
> https://commitfest.postgresql.org/action/patch_view?id=224 .
>
> Parametrised queries are supported in PostgreSQL long time. Using the
> parametrised queries is point of all advices about good programming
> style. On application level it is protection to SQL injection. On low
> level it is protection to some potential quoting or escaping bugs. It
> is paradox, so PostgreSQL doesn't use this techniques in own
> applications - mainly in psql.
>
> In psql we have not any quoting, or escaping functionality. We have to
> use external tools like awk, sed:
>
> http://www.redhat.com/docs/manuals/database/RHDB-2.1-Manual/admin_user/r1-app-psql-4.html
>
>>
>> testdb=> \set content '\'' `cat my_file.txt` '\''
>> testdb=> INSERT INTO my_table VALUES (:content);
>>
>> One possible problem with this approach is that my_file.txt might contain single quotes.
>> These need to be escaped so that they do not cause a syntax error when the
>> third line is processed. You can do this with the program sed:
>>
>> testdb=> \set content `sed -e "s/'/\\\\\\'/g" < my_file.txt`
>
> Similar problems could be removed with using parameter queries in psql.
>
> With this parametrised queries feature you can:
>
> \set content `cat my_file.txt`
> INSERT INTO my_table VALUES(:content);
>
> and this command will be correct without depending on content my_file.txt file.
>
> This is more: robust, secure, and simpler.
>
> My motivation is simplify life to people who use psql for scripting.
> For internal use SQL injection isn't too much terrible. Problem are
> some obscure identifiers used some users. Now you have to carefully
> check every value, if your scripts have to be robust.
>
> Patch doesn't change default behave. You have to explicitly activate it.

This makes sense now that you've explained it.  Personally, I would
not choose to use psql as a scripting language, and I think there has
been some controversy on that point in the past, though I don't
remember the details.  In spite of that, though, it seems to me that
it does make some sense to provide a mechanism for escaping the value
stored in a psql variable, since - if nothing else - someone might
easily want to do the sort of thing you're describing here in an
interactive session.

However, I think the approach you've taken in this patch is a
non-starter.  You've basically added a global flag that will cause ALL
variables to be passed in a way that removes the need for them to be
escaped.  That seems pretty inconvenient and awkward.  What happens if
someone wants to do "INSERT INTO :foo VALUES (:bar)"?  They're out of
luck.  Futhermore, if a psql script that expects the pexec flag to be
set one way is run with it set the other way, it may either work fine,
work OK but with a potential security hole, or fail spectacularly.  I
think maybe what we need here is a piece of syntax to indicate that a
specific parameter should be substituted after first being passed
through PQescapeStringConn.

Other thoughts?

...Robert


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Small change of the HS document
Next
From: Bruce Momjian
Date:
Subject: Re: Removing pg_migrator limitations