Re: info about patch: using parametrised query in psql

On Thu, Dec 24, 2009 at 2:45 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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