Thread: SQL injection, php and queueing multiple statement

Is there a switch (php side or pg side) to avoid things like:

pg_query("select id from table1 where a=$i");

into becoming

pg_query("select id from table1 where a=1 and 1=1; do something
nasty; -- ");

So that every
pg_query(...) can contain no more than one statement?

thanks

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

> Is there a switch (php side or pg side) to avoid things like:
>
> pg_query("select id from table1 where a=$i");
>
> into becoming
>
> pg_query("select id from table1 where a=1 and 1=1; do something
> nasty; -- ");

Ideally, you'd use this:

pg_query_params('select id from table1 where a=$1', array($i));

http://us2.php.net/manual/en/function.pg-query-params.php

Alternately, you can do this:

$i = pg_escape_string($i);
pg_query(" select id from table1 where a='$i' ");

mail@webthatworks.it (Ivan Sergio Borgonovo) writes:
> Is there a switch (php side or pg side) to avoid things like:
>
> pg_query("select id from table1 where a=$i");
>
> into becoming
>
> pg_query("select id from table1 where a=1 and 1=1; do something
> nasty; -- ");
>
> So that every
> pg_query(...) can contain no more than one statement?

The conventional approach to this sort of thing is to use prepared
statements:

http://ca3.php.net/manual/en/function.pg-prepare.php

In effect, you set up the query beforehand, pre-parameterizing.

<?php
// Connect to a database named "mary"
$dbconn = pg_connect("dbname=mary");

// Prepare a query for execution
$result = pg_prepare($dbconn, "my_query", 'SELECT * FROM shops WHERE name = $1');

// Execute the prepared query.  Note that it is not necessary to escape
// the string "Joe's Widgets" in any way
$result = pg_execute($dbconn, "my_query", array("Joe's Widgets"));

// Execute the same prepared query, this time with a different parameter
$result = pg_execute($dbconn, "my_query", array("Clothes Clothes Clothes"));

?>

Assuming that PHP is actually using PostgreSQL prepared statements
(and not just faking things behind your back), this should nicely
address the problem of injection attacks.
--
(reverse (concatenate 'string "ofni.sesabatadxunil" "@" "enworbbc"))
http://linuxfinances.info/info/linuxdistributions.html
The  average woman would rather   have beauty than  brains because the
average man can see better than he can think.

On Fri, 11 Apr 2008 14:27:09 -0500
"Adam Rich" <adam.r@sbcglobal.net> wrote:

> > Is there a switch (php side or pg side) to avoid things like:
> >
> > pg_query("select id from table1 where a=$i");
> >
> > into becoming
> >
> > pg_query("select id from table1 where a=1 and 1=1; do something
> > nasty; -- ");
>
> Ideally, you'd use this:
>
> pg_query_params('select id from table1 where a=$1', array($i));
>
> http://us2.php.net/manual/en/function.pg-query-params.php
>
> Alternately, you can do this:
>
> $i = pg_escape_string($i);
> pg_query(" select id from table1 where a='$i' ");

I'd try to be clearer.
The purpose of my question was not how to avoid sql injection... but
how to make it harder to exploit it.
My premise is that someone will do mistakes in the php code and I'd
like to mitigate the effect of these mistakes.

I know that even if you just permit one statement for each pg_query
you can still use blind sql injection to download whole tables etc...
but permitting more than one statement make things MUCH easier.

Up to my knowledge blind sql injection requires a lot of statement
and a lot of errors that will end up in my logs so I'll have a chance
to fix the error etc...

Prepared statements does not fit with part of the framework I'm
working with. And still I'm looking for a security net even in the
case someone is not respecting the policies.

thx

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

"My premise is that someone will do mistakes in the php code and I'd like to
mitigate the effect of these mistakes."

- Prepared statements is the only "bulletproof" technique

- You can use a database abstraction layer (there are more than many
libraries for PHP) Fast to implement, all queries goes through some form of
filter

- Recommended solution - use database abstraction & AUDIT your code / grabs
all SQL statements / create a summary and make sure then each statement is
safe

$db->Execute("select id from table1 where integer =" (int)$i);
$db->Execute("select * from table1 where string =" $db->quote($i));

Database abstractions alone often give you a false sense of security
especially if you use third part / open source solutions

-----Original Message-----
From: pgsql-general-owner@postgresql.org
[mailto:pgsql-general-owner@postgresql.org] On Behalf Of Ivan Sergio
Borgonovo
Sent: April 11, 2008 5:32 PM
To: pgsql-general@postgresql.org
Subject: Re: [GENERAL] SQL injection, php and queueing multiple statement

On Fri, 11 Apr 2008 14:27:09 -0500
"Adam Rich" <adam.r@sbcglobal.net> wrote:

> > Is there a switch (php side or pg side) to avoid things like:
> >
> > pg_query("select id from table1 where a=$i");
> >
> > into becoming
> >
> > pg_query("select id from table1 where a=1 and 1=1; do something
> > nasty; -- ");
>
> Ideally, you'd use this:
>
> pg_query_params('select id from table1 where a=$1', array($i));
>
> http://us2.php.net/manual/en/function.pg-query-params.php
>
> Alternately, you can do this:
>
> $i = pg_escape_string($i);
> pg_query(" select id from table1 where a='$i' ");

I'd try to be clearer.
The purpose of my question was not how to avoid sql injection... but
how to make it harder to exploit it.
My premise is that someone will do mistakes in the php code and I'd
like to mitigate the effect of these mistakes.

I know that even if you just permit one statement for each pg_query
you can still use blind sql injection to download whole tables etc...
but permitting more than one statement make things MUCH easier.

Up to my knowledge blind sql injection requires a lot of statement
and a lot of errors that will end up in my logs so I'll have a chance
to fix the error etc...

Prepared statements does not fit with part of the framework I'm
working with. And still I'm looking for a security net even in the
case someone is not respecting the policies.

thx

--
Ivan Sergio Borgonovo
http://www.webthatworks.it


--
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

On Sat, 12 Apr 2008 11:11:48 -0400
"Jonathan Bond-Caron" <jbondc@gmail.com> wrote:

> "My premise is that someone will do mistakes in the php code and
> I'd like to mitigate the effect of these mistakes."
>
> - Prepared statements is the only "bulletproof" technique

I'm not looking for something bullet proof, I'm looking for one more
mitigating factor.
I already use an "fprint" technique that cast or escape sql input
before passing it to pg_query... but being able to queue 2 statements
make it much easier to reach whatever target the attacker may have.

I hate to make comparisons but up to my knowledge:
- MySQL drivers for php don't let you queue more than one statement
in a query (that's not the right solution)
- MS SQL has a server switch that let you chose if you'd like to have
more than one statement or not

As said... there are blind sql injection techniques that once the
door is open will let the attacker download the whole content of a DB.
But generally that requires time and it is achieved causing sql
errors. Errors get logged. Log analysis may spot the problem before
it is too late.
I know that other techniques rely on measuring execution time... but
well you've already put the bar much higher than letting slip in

select id, name from table1 where id=7 and 1=1; drop table table2; --

Somehow the sql statement is parsed along the way and you can't just
skip everything that is past a ; since

select id, name from table1 where name ilike '%;';

is a legit statement.

And it should be a switch... since you may need to load functions
and functions generally contain multiple statement separated by ; or
you may have to execute more than one statement in one transaction...
Isolating the code that can issue multiple statement from the one
that can't would come handy.

Is there such a switch in the php driver for pg? or is it in the
server cfg? If there is not such a thing it would be nice to have it.

I may sound naive but having a way to protect the DB from this kind
of injections looks as a common problem, I'd thought there was
already a common solution.
If there is not such a switch is there a reason?

> - Recommended solution - use database abstraction & AUDIT your
> code / grabs all SQL statements / create a summary and make sure
> then each statement is safe

While this are other barriers to fight coding errors they have a much
higher cost than just being able to block queued statement.

> Database abstractions alone often give you a false sense of security
> especially if you use third part / open source solutions

It is exactly that false sense of security I'm trying to fight,
placing several barriers on the way of a potential attacker.

Being able to stop queued statements seemed a cheap barrier but with a
reasonably good ROI.

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

Ivan Sergio Borgonovo <mail@webthatworks.it> writes:
> I may sound naive but having a way to protect the DB from this kind
> of injections looks as a common problem, I'd thought there was
> already a common solution.

Use prepared statements.

            regards, tom lane

On Sat, 12 Apr 2008 12:39:38 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Ivan Sergio Borgonovo <mail@webthatworks.it> writes:
> > I may sound naive but having a way to protect the DB from this
> > kind of injections looks as a common problem, I'd thought there
> > was already a common solution.
>
> Use prepared statements.

Yeah... but how can I effectively enforce the policy that ALL input
will be passed through prepared statements?

If I can't, and I doubt there is a system that will let me enforce
that policy at a reasonable cost, why not providing a safety net that
will at least raise the bar for the attacker at a very cheap cost?

If programmers didn't make errors or errors where cheap to find there
wouldn't be any sql injection problem.

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

On Fri, Apr 11, 2008 at 9:21 PM, Ivan Sergio Borgonovo
<mail@webthatworks.it> wrote:
> Is there a switch (php side or pg side) to avoid things like:
>
>  pg_query("select id from table1 where a=$i");
>
>  into becoming
>
>  pg_query("select id from table1 where a=1 and 1=1; do something
>  nasty; -- ");
>
>  So that every
>  pg_query(...) can contain no more than one statement?

Well, use prepared statements.

Apart from that, make it impossible to "do something nasty".  Your
php_db_user should be
allowed as little as possible.  Specifically:
 * she should not be owner of the tables/other objects -- this way you are safe
from nasty "DROP TABLE"s and the like.
 * you should decide where she is allowed to INSERT/UPDATE/DELETE, the latter
two are the most dangerous ones.
 * you should make use of referential integrity constraints -- so evil
DELETE or UPDATE
will probably fail on these. ;)
 * you should provide PL/pgSQL stored procedures to update your vital
data.  So evil
bulk delete/update will be harder to accomplish (if your evildoer can
craft exploit to
do it, he probably already has a lot of access to your system ;)).

...oh and think about isolating read-only acces (read only user) from
rw-user -- if
that sounds reasonable to do so.

   Regards,
      Dawid

Ivan Sergio Borgonovo wrote:
> Yeah... but how can I effectively enforce the policy that ALL input
> will be passed through prepared statements?
>

Code reviews are about the only way to enforce this.


> If I can't, and I doubt there is a system that will let me enforce
> that policy at a reasonable cost, why not providing a safety net that
> will at least raise the bar for the attacker at a very cheap cost?
>

How do you do this? Disallow string concatenation and/or variable
interpolation for any string that's going to be shipped off to the
database? Do you parse the SQL string according to the rules of any
backend database you might be talking to, to see if you have a where
clause not using a prepared statement? i.e. - Nothing is going to work here.

You're stuck with making sure developers know the most rudimentary
things about talking to a database.

paul rivers wrote:
> Ivan Sergio Borgonovo wrote:
>> Yeah... but how can I effectively enforce the policy that ALL input
>> will be passed through prepared statements?
>>
>
> Code reviews are about the only way to enforce this.

That's not entirely true - if you have a policy that says thou-shalt-not-use
non-prepared statements then an automated (nightly) grep of the source tree is
going to find calls that don't follow that policy - these can be automatically
emailed to you/your project enforcer. Couple that with a process step that
enforces a grep for offending statements as part of transferring files to the
live servers and you're about done.

I'm assuming that the current  policy would not allow just anyone to upload any
code to critical live systems anyway.

>
>
>> If I can't, and I doubt there is a system that will let me enforce
>> that policy at a reasonable cost, why not providing a safety net that
>> will at least raise the bar for the attacker at a very cheap cost?
>>
>
> How do you do this? Disallow string concatenation and/or variable
> interpolation for any string that's going to be shipped off to the
> database? Do you parse the SQL string according to the rules of any
> backend database you might be talking to, to see if you have a where
> clause not using a prepared statement? i.e. - Nothing is going to work
> here.
>
> You're stuck with making sure developers know the most rudimentary
> things about talking to a database.
>
>
>

"paul rivers" <rivers.paul@gmail.com> writes:

>> If I can't, and I doubt there is a system that will let me enforce
>> that policy at a reasonable cost, why not providing a safety net that
>> will at least raise the bar for the attacker at a very cheap cost?
>
> How do you do this? Disallow string concatenation and/or variable interpolation
> for any string that's going to be shipped off to the database?

Actually there is a system that can do this. Perl with the -T option. It keeps
track of which strings are "tainted" by user-input and functions like eval
will cause errors if you try to pass them a tainted string. The database
drivers support this and will trigger an error if they're passed a tainted
string.

It doesn't protect you from a malicious programmer of course. It's easy to
intentionally untaint a string. But it does a pretty good job of protecting
you against accidental security holes.

In my experience if you have a flat policy of _never_ allowing interpolated
strings then the problem just doesn't come up. It's easy to spot interpolated
strings during code review. If you have a more complex policy where sometimes
they're allowed if they come from internal data structures or they've been
checked then I think it would be helpful.

--
  Gregory Stark
  EnterpriseDB          http://www.enterprisedb.com
  Ask me about EnterpriseDB's 24x7 Postgres support!

Gregory Stark wrote:
> "paul rivers" <rivers.paul@gmail.com> writes:
>
>
>>> If I can't, and I doubt there is a system that will let me enforce
>>> that policy at a reasonable cost, why not providing a safety net that
>>> will at least raise the bar for the attacker at a very cheap cost?
>>>
>> How do you do this? Disallow string concatenation and/or variable interpolation
>> for any string that's going to be shipped off to the database?
>>
>
> Actually there is a system that can do this. Perl with the -T option. It keeps
> track of which strings are "tainted" by user-input and functions like eval
> will cause errors if you try to pass them a tainted string. The database
> drivers support this and will trigger an error if they're passed a tainted
> string.
>
>

Good point. What happens in the case I query a string from the database,
and use this result to build another sql string via concatenation?
Assume the value in the database came from user input, albeit via
another source and not this script. Will taint catch this? (Genuine
question - I don't know.)

Ivan Sergio Borgonovo <mail@webthatworks.it> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Use prepared statements.

> Yeah... but how can I effectively enforce the policy that ALL input
> will be passed through prepared statements?

Modify the PHP code (at whatever corresponds to the DBD layer)
to always use PQexecParams, never PQexec, even when you don't
have any parameters.

            regards, tom lane

On Sat, 12 Apr 2008 20:25:36 +0100
Gregory Stark <stark@enterprisedb.com> wrote:

> "paul rivers" <rivers.paul@gmail.com> writes:
>
> >> If I can't, and I doubt there is a system that will let me
> >> enforce that policy at a reasonable cost, why not providing a
> >> safety net that will at least raise the bar for the attacker at
> >> a very cheap cost?
> >
> > How do you do this? Disallow string concatenation and/or variable
> > interpolation for any string that's going to be shipped off to
> > the database?
>
> Actually there is a system that can do this. Perl with the -T
> option. It keeps track of which strings are "tainted" by user-input
> and functions like eval will cause errors if you try to pass them a
> tainted string. The database drivers support this and will trigger
> an error if they're passed a tainted string.

The db driver?
Could you make any real php example?

eg. I'm looking to provide a safety net to:

$querystring='select id, name from table1 where id in
(".$somearray.") and name like $1';
$result=pg_prepare($dbconn, "a_query",$querystring);

On Sat, 12 Apr 2008 16:18:34 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Modify the PHP code (at whatever corresponds to the DBD layer)
> to always use PQexecParams, never PQexec, even when you don't
> have any parameters.

How it is going to work for the above case?
I read:
"The primary advantage of PQexecParams over PQexec is that parameter
values may be separated from the command string, thus avoiding the
need for tedious and error-prone quoting and escaping. Unlike PQexec,
PQexecParams allows at most one SQL command in the given string.
(There can be semicolons in it, but not more than one nonempty
command.) This is a limitation of the underlying protocol, but has
some usefulness as an extra defense against SQL-injection attacks."

Are you suggesting that wrapping all the pg_query into pg_prepare +
pg_execute will solve the problem? And that I kept misunderstanding
what you were saying in the previous N posts? ;)

that means that if an attacker push more than one statement trough
$somearray in the above example it won't work?

But what about already written code that use pg_query?
Is it that terrible or nonsensical to hope to have a switch that will
enable/disable multiple statements for each call to pg_query?

On Sat, 12 Apr 2008 20:52:11 +0200
"Dawid Kuroczko" <qnex42@gmail.com> wrote:

[some good advices]

Yep... I'm already doing my best at it.
Working with libraries doesn't make it always feasible and it is far
more complicated than just forbidding extra statement in each call to
pg_query.

thx

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

On Sat, Apr 12, 2008 at 11:06:42PM +0200, Ivan Sergio Borgonovo wrote:
> But what about already written code that use pg_query?

If you rewrite the database interface then it doesn't matter, the calls
to pg_query will end up being calls to prepare/execute underneath so
you'll have their protection.  If you mean that you're calling compiled
code that is already statically linked to a library that calls pg_query
then you may get some mileage out of modifying some proxying software
(which you'll probably be using anyway if you're doing any web based
stuff) to rewrite all the calls rather than doing it between PHP and the
driver.  Anyway I'm not sure if this'll help you much, you have to put
trust in your code somewhere.  It's just a question of where.

> Is it that terrible or nonsensical to hope to have a switch that will
> enable/disable multiple statements for each call to pg_query?

I agree that it will close some, probably small, attack vectors, but
that in general it's not going to help much.  I can't help but think
that you're trying to solve what's formally known as the "Halting
Problem" (or originally the Entscheidungsproblem) which was shown to be
impossible in the 30s.

Even if you did close this hole, what's to stop the attacker from
exploiting another bug in your code and causing your code to misbehave
in some other way.  About the only tool known by software engineering
that will solve this problem is that of formally methodology.  You have
to sit down and spend a long time modelling your problem, then you
have to prove (formally, i.e. in some logic system) that your model is
correct, then you write the code based on the model, then you prove that
your code is true to the model.  This is a *lot* of work and is only
done for very high assurance stuff at the moment, aeroplane control
software being one place it's used.

If you want an easier way of getting close to a good solution then you
want tools that will check that the code you've written is correct as
early as possible.  I personally think, and there's a lot of research
saying the same thing, that statically-typed and safe languages with
formally defined semantics are good tools here.  PHP is a tool for
writing code quickly, not for writing high assurance code.


  Sam

paul rivers wrote:
> Ivan Sergio Borgonovo wrote:
>> Yeah... but how can I effectively enforce the policy that ALL input
>> will be passed through prepared statements?
>>
>
> Code reviews are about the only way to enforce this.
>
(Note: I'm clueless about PHP, so I'm basing this on perl/python/etc):
Something that'll make code review easier is to wrap the DB driver with
your own module that performs the checks you're looking to do on the
SQL. Prohibit use of the postgresql db interface module directly.
Anybody who directly imports the postgresql driver is doing something
wrong in a way that's easily detectable. You could even, with a little
effort, use a svn hook script to scan for and reject commits including
the import statement in question, though a deliberately malicious
programmer could still get around that.

Your wrapper code can potentially do things like scan a string for
semicolons not enclosed in single or double quotes. The rule probably
has to be a little more complex than that, and has to handle escaped
quotes, but it might achieve what you want.

Personally I doubt it's worth it for the questionable benefits gained
over following a strong coding standard and having good code review.

I do think that for web scripting users it would be nice for DB drivers
to optionally throw an error if an execute call passes more than one
statement. Your problem would be that it'd need to be an option that
affects the whole app instance for it to achieve what you want without
developer action, and a global option like that would potentially break
3rd party application code. The alternative would be something like an
executeSingle(...) call or a flag to execute ... but that again just
comes back to proper code review to ensure it's used.

--
Craig Ringer

On Sun, 13 Apr 2008 10:03:48 +0800
Craig Ringer <craig@postnewspapers.com.au> wrote:

> Your wrapper code can potentially do things like scan a string for
> semicolons not enclosed in single or double quotes. The rule
> probably has to be a little more complex than that, and has to
> handle escaped quotes, but it might achieve what you want.

I think this logic is already somewhere in the driver or the pg
engine. Whatever you write at the application level a) risk to be a
duplication of part of the parser b) risk to be less smart than the
parser itself and let slip something.

> Personally I doubt it's worth it for the questionable benefits
> gained over following a strong coding standard and having good code
> review.

> I do think that for web scripting users it would be nice for DB
> drivers to optionally throw an error if an execute call passes more
> than one statement. Your problem would be that it'd need to be an
> option that affects the whole app instance for it to achieve what
> you want without developer action, and a global option like that
> would potentially break 3rd party application code. The alternative
> would be something like an executeSingle(...) call or a flag to
> execute ... but that again just comes back to proper code review to
> ensure it's used.

Why does it have to be "global", couldn't it be "by connection" or
"by call to pg_query"?

On Sun, 13 Apr 2008 00:13:49 +0100
Sam Mason <sam@samason.me.uk> wrote:

> On Sat, Apr 12, 2008 at 11:06:42PM +0200, Ivan Sergio Borgonovo
> wrote:
> > But what about already written code that use pg_query?
>
> If you rewrite the database interface then it doesn't matter, the
> calls to pg_query will end up being calls to prepare/execute
> underneath so you'll have their protection.  If you mean that

Sorry to everybody for not testing first what you were suggesting in
chorus.

function test_me() {
    $result=pg_prepare("tonno","
        select ItemID from catalog_items limit 1;
        create table tonno(au int);");
    $result=pg_execute("tonno");
    return "pippo";
}

Query failed: ERROR: cannot insert multiple commands into a prepared
statement in ...

Not something easy to deploy after the cats are out, but at least I've
learnt something.

thanks

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

Ivan Sergio Borgonovo wrote:
> On Sun, 13 Apr 2008 10:03:48 +0800
> Craig Ringer <craig@postnewspapers.com.au> wrote:
>
>
>> Your wrapper code can potentially do things like scan a string for
>> semicolons not enclosed in single or double quotes. The rule
>> probably has to be a little more complex than that, and has to
>> handle escaped quotes, but it might achieve what you want.
>>
>
> I think this logic is already somewhere in the driver or the pg
> engine. Whatever you write at the application level a) risk to be a
> duplication of part of the parser b) risk to be less smart than the
> parser itself and let slip something.
>
... in which case it sounds like you need to extend the Pg DB interface
to do what you want. It might be worth hacking together a proof of
concept and posting it to -hackers and the PHP interface maintainers,
along with a rationale for its inclusion.
>> I do think that for web scripting users it would be nice for DB
>> drivers to optionally throw an error if an execute call passes more
>> than one statement. Your problem would be that it'd need to be an
>> option that affects the whole app instance for it to achieve what
>> you want without developer action, and a global option like that
>> would potentially break 3rd party application code. The alternative
>> would be something like an executeSingle(...) call or a flag to
>> execute ... but that again just comes back to proper code review to
>> ensure it's used.
>>
>
> Why does it have to be "global", couldn't it be "by connection" or
> "by call to pg_query"?
>
Because you appear to be seeking something to protect against
programmers who do not follow coding guidelines, and that should help
even if code review processes fail to catch the problem. Were that not
the case you'd be able to use some of the other suggestions made here. I
quote:
>
> Yeah... but how can I effectively enforce the policy that ALL input
> will be passed through prepared statements?
>
> If I can't, and I doubt there is a system that will let me enforce
> that policy at a reasonable cost, why not providing a safety net that
> will at least raise the bar for the attacker at a very cheap cost?
>
> If programmers didn't make errors or errors where cheap to find there
>
How can you ensure that programmers will set the single statement flag
on each connection or set the flag on each query? If you can do those
things, you can also ensure that they just follow string handling rules,
use prepared statements where possible, require additional review of all
non-prepared-statement code, etc.

An app instance wide flag at least ensure that it's set once (say, in
PHP configuration) and *stays* set, no matter what stupid things
individual developers might do. For more general use a per-query and
per-connection flag would be useful, but it doesn't sound like it would
fit your specific needs unless you can hide the DB interface behind a
connection factory that you can ensure will always set the option. You
could also have a per-query-call flag to permit multiple statements;
it'd be something for library authors to use where required no matter
what the connection and global defaults were, ensuring they always got
correct behaviour. Such a flag would also be easy to scan for in
automated code review or in a revision control system hook script.

Also, as already noted there are ways to help you enforce the use of
prepared statements. If you have a decent code review process in place
then you probably have a `lint' script that's run as a first pass to
spot possible problems in the code. You can extend that script, and also
hook it into the revision control system so it emails you (or rejects
the commit) if somebody tries to commit code that looks worrying, like
non-prepared-query database interface calls. A simple annotation scheme
should permit you to ignore the legit ones, with unauthorized addition
of annotations punishable by cold coffee for a year.

There's probably an existing PHP lint script you can extend, so you
don't have to do the boring bits. A quick search suggests so.

Ideally you want a lint script or PHP extension that can also do static
and/or runtime analysis for user input `taint', like the Perl option of
the same name. A Google search for `php taint' suggests that there are
certainly efforts in that direction, though not being active in PHP it's
hard for me to tell how complete or useful they are.

In any case, I agree with you that a "single statement only" flag would
be nice in the DB interface, because as you say it's nice if all else
fails and will block a many of the most flexible types of SQL injection
attack. I just think that if it exists it needs to be opt-out, not
opt-in, to be significantly effective as a defense against other
programming errors.

--
Craig Ringer

On Sun, 13 Apr 2008 16:02:35 +0800
Craig Ringer <craig@postnewspapers.com.au> wrote:

> > I think this logic is already somewhere in the driver or the pg
> > engine. Whatever you write at the application level a) risk to be
> > a duplication of part of the parser b) risk to be less smart than
> > the parser itself and let slip something.

> ... in which case it sounds like you need to extend the Pg DB
> interface to do what you want. It might be worth hacking together a
> proof of concept and posting it to -hackers and the PHP interface
> maintainers, along with a rationale for its inclusion.

I wish I'd be so familiar with pg C code.
And it looks as if such a thing won't be that welcome.
Everybody seems happy with pg_prepare|execute and consider not using
them shameful enough not to be worth some form of "legacy support".

> > Why does it have to be "global", couldn't it be "by connection" or
> > "by call to pg_query"?

> Because you appear to be seeking something to protect against
> programmers who do not follow coding guidelines, and that should
> help even if code review processes fail to catch the problem. Were
> that not the case you'd be able to use some of the other
> suggestions made here. I quote:

Default 1 statement, switch to more than one have to be "voluntary"
and "conscious" and can be easily spotted with grep only.

> Also, as already noted there are ways to help you enforce the use
> of prepared statements. If you have a decent code review process in
> place then you probably have a `lint' script that's run as a first
> pass to spot possible problems in the code. You can extend that
> script, and also hook it into the revision control system so it
> emails you (or rejects the commit) if somebody tries to commit code
> that looks worrying, like non-prepared-query database interface
> calls. A simple annotation scheme should permit you to ignore the
> legit ones, with unauthorized addition of annotations punishable by
> cold coffee for a year.

These things are nice... and I do as much as I can to deal with
security from different angles but they require more effort than
switching a flag.

> There's probably an existing PHP lint script you can extend, so you
> don't have to do the boring bits. A quick search suggests so.

These will be further steps as soon as I'll have some free time.
That said php as a language doesn't make easy to deal with static
checking and I had (?) the feeling many tools are still in their
infancy (eg. automatic refactoring tools that will make easier to wrap
all the calls of pg_query). But this is definitively OT for the
Postgresql list.

> In any case, I agree with you that a "single statement only" flag
> would be nice in the DB interface, because as you say it's nice if
> all else fails and will block a many of the most flexible types of
> SQL injection attack. I just think that if it exists it needs to be
> opt-out, not opt-in, to be significantly effective as a defense
> against other programming errors.

thanks

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

On Sun, Apr 13, 2008 at 10:37:52AM +0200, Ivan Sergio Borgonovo wrote:
> > Because you appear to be seeking something to protect against
> > programmers who do not follow coding guidelines, and that should
> > help even if code review processes fail to catch the problem. Were
> > that not the case you'd be able to use some of the other
> > suggestions made here. I quote:
>
> Default 1 statement, switch to more than one have to be "voluntary"
> and "conscious" and can be easily spotted with grep only.

It's not quite so simple, there are backward compatability issues.
Postgres supported multiple queries in one string since forever and
there is a huge amount of code written and still being written that
takes advantage of this. Most in contexts where SQL injection is simply
not an issue since they're static queries.

PQexec("BEGIN; CREATE TABLE ... ; CREATE TABLE ...; etc; COMMIT;");

For dynamic queries people were using prepared statements already so it
made sense to restrict it for prepared statements and leave one-shot
queries alone. You are advocating catering to a coding style that has
been discouraged for years.

If you want definitive answer I suppose you need to check the archives
at the time this decision was made, which is quite some time back.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while
> boarding. Thank you for flying nlogn airlines.

On Sun, 13 Apr 2008 11:49:58 +0200
Martijn van Oosterhout <kleptog@svana.org> wrote:

> On Sun, Apr 13, 2008 at 10:37:52AM +0200, Ivan Sergio Borgonovo
> wrote:
> > > Because you appear to be seeking something to protect against
> > > programmers who do not follow coding guidelines, and that should
> > > help even if code review processes fail to catch the problem.
> > > Were that not the case you'd be able to use some of the other
> > > suggestions made here. I quote:
> >
> > Default 1 statement, switch to more than one have to be
> > "voluntary" and "conscious" and can be easily spotted with grep
> > only.
>
> It's not quite so simple, there are backward compatability issues.

I'm aware of the problem. I couldn't use legacy as an argument just
to break other legacy stuff ;)
Actually I pointed out that giving no option is a bad idea, and
that's what mysql driver do, if I remember correctly.

I'd say default at the application level.

While it is pretty common to call pg_query directly, places where you
use pg_connect are fewer and generally is something less frequently
called directly and already wrapped into something that will load
connection parameters.
You'd switch multiple statement off (but still not at the
connection level) when you use pg_connect and if you want multiple
statements you'd have to turn it on before you issue a pg_query, and
turn it off afterwards.

Of course if pg_query is NEVER (or very seldom) called directly in the
code... you'd already have a wrapper to turn every pg_query into a
pg_prepare + pg_execute sequence.

I'm not here to ask anyone will implement my ideas in the postgres
driver for php ;) and from what I've learnt on pg_prepare/pg_execute
I've enough tools to mitigate the problem at least in MY code since
pg_query is NEVER called directly.
I thought that _prepare _execute was just a more conscious form of
fprint... while it is not. So I kept thinking that it was still
possible to inject multiple statements.

thanks to everybody who insisted enough to let me grasp what you were
writing by a long time.

--
Ivan Sergio Borgonovo
http://www.webthatworks.it

On Sun, 13 Apr 2008, Ivan Sergio Borgonovo wrote:

> On Sun, 13 Apr 2008 16:02:35 +0800
> Craig Ringer <craig@postnewspapers.com.au> wrote:
>
> > > I think this logic is already somewhere in the driver or the pg
> > > engine. Whatever you write at the application level a) risk to be
> > > a duplication of part of the parser b) risk to be less smart than
> > > the parser itself and let slip something.
>
> > ... in which case it sounds like you need to extend the Pg DB
> > interface to do what you want. It might be worth hacking together a
> > proof of concept and posting it to -hackers and the PHP interface
> > maintainers, along with a rationale for its inclusion.
>
> I wish I'd be so familiar with pg C code.
> And it looks as if such a thing won't be that welcome.

Well, Tom suggested making the PHP interface optionally use PQexecParams
rather than PQexec even when using a full query string with no parameters
as that interface doesn't support multiple queries, so I don't think it's
necessarily entirely unwelcome - of course, we're not the PHP team, so
they might view it differently.

One issue is that it appears that PHP's interface tries to support cases
where the libpq version doesn't have PQexecParams, and you'd probably be
best to follow the existing style, only using PQexecParams if
HAVE_PQEXECPARAMS and the configuration option is set.

There appear to be 15 calls to PQexec inside the PHP ext/pgsql.c for the
version I have of PHP. 7 of them appear to use a constant string in the
call, so don't necessarily need to change. A few of the others are
generated single queries for metadata and the like and probably don't need
to be configurable to allow multiple queries but merely on
HAVE_PQEXECPARAMS.

Developers,

It seems you are overlooking application user/system admin perspective.

I agree developers should use prepared statement, but application user or
system admins are not able to modify codes usually.

There are many PostgreSQL/MySQL applications that generating SQL statements.

MySQL's query API only allow single SQL statements at a time, while PostgreSQL
allows multiple statements. Therefore, MySQL users will likely to have less
impact from buggy codes compare to PostgreSQL users.

It would be nice to have API like PQquerySingle that allows only a single SQL
statement at a time.

--
Yasuo Ohgaki

Yasuo Ohgaki <yohgaki@ohgaki.net> writes:
> It would be nice to have API like PQquerySingle that allows only a single SQL
> statement at a time.

We have one (the "extended query" protocol).

            regards, tom lane