Protection from SQL injection

Thomas,

What do you think about it? Do you think it makes sense to implement
this security feature in PostgreSQL as well? If not why not? Does
PostgreSQL have another solution or plan to solve the SQL injection
problem?

Have you seen Meredith's libdejector?
http://sourceforge.net/projects/libdejector

--Josh Berkus

* Thomas Mueller (thomas.tom.mueller@gmail.com) wrote:

As you know, "SQL injection" is the main security problem of databases today.

I think there's a fallacy there- it's the main security problem of
applications (particularly those on the web) today. It hasn't got much
at all to do with the database's security.

Also, hasn't this been discussed to death already?

Stephen

As you know, "SQL injection" is the main security problem of databases
today.

I think I found a solution: 'disabling literals'. Or you may call it
'enforcing the use of parameterized statements'. This means that SQL
statements with embedded user input are rejected at runtime. My
solution goes beyond saying "developers ~should~ use parameterized
statements". That is not a solution because developers are lazy. My
solution is: "developers MUST use parameterized statements". It goes
like this: Literals are disabled using the SQL statement:

I have found that the little bit of code posted afterwards did eliminate
SQL holes in my PHP applications with zero developer pain, actually it is
MORE convenient to use than randomly pasting strings into queries.

You just call
db_query( "SELECT * FROM table WHERE column1=%s AND column2=%s",
array( $var1, $var2 ));

It is inspired from the Python interface which performs the same (but
slightly more elegantly).
I have removed the logging features for clarity.

function db_quote_query( $sql, $params=false )
{
// if no params, send query raw
if( $params === false ) return $sql;
if( !is_array( $params )) $params = array( $params );

// quote params
foreach( $params as $key => $val )
{
if( is_array( $val )) $params[$key] = implode( ', ', array_map( intval,
$val ));
else $params[$key] =
is_null($val)?'NULL':("'".pg_escape_string($val)."'");;
}
return vsprintf( $sql, $params );
}

function db_query( $sql, $params=false )
{
// it's already a query
if( is_resource( $sql ))
return $sql;

$sql = db_quote_query( $sql, $params );

$r = pg_query( $sql );
if( !$r )
{
echo "<div class=bigerror><b>Erreur PostgreSQL :</b><br
/>".htmlspecialchars(pg_last_error())."<br /><br /><b>Requête</b> :<br
/>".$sql."<br /><br /><b>Traceback </b>:<pre>";
foreach( debug_backtrace() as $t ) xdump( $t );
echo "</pre></div>";
die();
}
return $r;
}

On Tue, Apr 29, 2008 at 7:00 AM, PFC <lists@peufeu.com> wrote:

I have found that the little bit of code posted afterwards did eliminate
SQL holes in my PHP applications with zero developer pain, actually it is
MORE convenient to use than randomly pasting strings into queries.

You just call
db_query( "SELECT * FROM table WHERE column1=%s AND column2=%s", array(
$var1, $var2 ));

Implementing this for yourself is crazy; PHP's Postgres extension
already does this for you since 5.1.0:

$result = pg_query_params("SELECT foo FROM bar WHERE baz = $1", array($baz));

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

Cheers,
BJ

On Mon, Apr 28, 2008 at 08:55:34PM +0200, Thomas Mueller wrote:

As you know, "SQL injection" is the main security problem of databases today.

I think I found a solution: 'disabling literals'.

I personally think this is wrong, I often have schemas that mean I have
to do things like:

SELECT a.x, a.y, b.z
FROM a, b
WHERE a.a = b.a
AND a.f = 'lit'
AND b.g = 'lit'
AND b.h = $1;

So a big query, with lots of literals and only very few of them actually
come from an untrusted source. Also remember that any literal (i.e. not
just strings) can be quoted, think of dates in queries.

One option I like would be if the programming language (that you're
calling the database from) recorded "tainting" of variables, preferably
if this is done statically in the type system but languages like PHP
seem to prefer to do this sort of thing at run time.

Microsoft's approach of integrating SQL into the language would work as
well, the programmer can't get the quoting wrong then. But I prefer the
approach taken by HaskellDB as it doesn't require new syntax/semantics
to be designed/integrated. HaskellDB is a bit heavy though.

Sam

On Tue, 29 Apr 2008 01:03:33 +0200, Brendan Jurd <direvus@gmail.com> wrote:

On Tue, Apr 29, 2008 at 7:00 AM, PFC <lists@peufeu.com> wrote:

I have found that the little bit of code posted afterwards did
eliminate
SQL holes in my PHP applications with zero developer pain, actually it
is
MORE convenient to use than randomly pasting strings into queries.

You just call
db_query( "SELECT * FROM table WHERE column1=%s AND column2=%s", array(
$var1, $var2 ));

Implementing this for yourself is crazy; PHP's Postgres extension
already does this for you since 5.1.0:

$result = pg_query_params("SELECT foo FROM bar WHERE baz = $1",
array($baz));

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

Cheers,
BJ

pg_query_params is quite slower actually...

Hi,

Meredith's libdejector

1) The last activity was 2005-12-17 :-(
2) From the docs: "the techniques used ... are ... being explored for
patentability".
3) The tool validates the SQL statement. This is not required when
using parameterized queries.
4) An 'exemplar' query is required for each query.
It's an interesting idea, and can even find the ORDER BY injection
that 'disabling literals' can't find. However there are problems: 2) +
4).

zero developer pain

Actually it's not zero pain, but the main problem is: there is no way
to enforce using it.

[SQL injection] is the main security problem of applications

Yes and no. Is buffer overflow an application or language problem? In
C / C++ buffer overflow is a problem. Java enforces array bounds
checking. What I suggest is to enforce using parameterized statements.
This is like having a painless, enforcible 'array bounds checking
mode' in C / C++.

hasn't this been discussed to death already?

Yes, but no good solution has been found so far.

II have to do things like: WHERE a.f = 'lit' AND b.h = $1;

In C the best practice is to use #define for constants. In C++ you
have 'const', in Java 'static final'. Unfortunately the 'named
constant' concept doesn't exist in SQL. I think that's a mistake. I
suggest to support CREATE CONSTANT ... VALUE ... and DROP CONSTANT
..., example: CREATE CONSTANT STATE_ACTIVE VALUE 'active'.

any literal (i.e. not just strings) can be quoted, think of dates in queries.

The problem is not only quotes. The problem is all kinds of user
input. For example: sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " +
orderId; This is not a problem if orderId is a number. But what if
it's a String? For example "1 AND (SELECT * FROM USERS WHERE
NAME='admin' AND PASSWORD LIKE 'm%')". An attacker could then retrieve
the admin password quite quickly.

"tainting" of variables

See Meredith's libdejector: regular expression checking doesn't always
work. Also, programming languages such as Java don't support tainting.
And it's again in the hand of the developer to use it, not use it, or
use it in the wrong way. There should be a way for an admin to enforce
using it, and using it correctly.

Microsoft's approach of integrating SQL into the language

Yes, LINQ is a good approach. For Java there is a project called
'Quaere' that provides something similar (however only when using the
'Alias' syntax, I wrote this part, see
http://svn.codehaus.org/quaere/trunk/Quaere/src/test/java/org/quaere/alias/test/SamplesOrderByTest.java).
However it will take a long time until all applications are converted.
With 'disabling literals', applications can be converted step-by-step.
'Disabling literals' can be used as a development tool, and it can be
enabled or disabled at runtime. With LINQ / Quaere / HaskellDB
migration will be harder and slower because you need to re-write the
application.

HaskellDB

The query syntax seems to be quite 'different'. I would prefer if the
syntax is as close as possible to SQL to simplify migration.

Regards,
Thomas

On Tue, Apr 29, 2008 at 01:37:37PM +0200, Thomas Mueller wrote:

any literal (i.e. not just strings) can be quoted, think of dates in queries.

The problem is not only quotes. The problem is all kinds of user
input. For example: sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " +
orderId; This is not a problem if orderId is a number. But what if
it's a String? For example "1 AND (SELECT * FROM USERS WHERE
NAME='admin' AND PASSWORD LIKE 'm%')". An attacker could then retrieve
the admin password quite quickly.

In other words, your programmer was stupid. And your example doesn't
work because no matter what the string is it can't return anything
other than rows from the orders table. If you're worried about them
using semicolons to introduce another query, prepare has prohibited
that for a long time already.

But as far as I'm concerned, the real killer is that it would make
using any interactive query interface impossible. I don't think it's
reasonable to include a complete SQL parser into psql just so I can
type normal queries.

Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

zero developer pain

Actually it's not zero pain, but the main problem is: there is no way
to enforce using it.

Sure, there is no way to enforce it (apart from grepping the source for
pg_query() and flogging someone if it is found), but is it really
necessary when the right solution is easier to use than the wrong solution
? Capitalizing on developer laziness is a win IMHO, lol.

The problem is not only quotes. The problem is all kinds of user
input. For example: sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " +
orderId; This is not a problem if orderId is a number. But what if
it's a String? For example "1 AND (SELECT * FROM USERS WHERE
NAME='admin' AND PASSWORD LIKE 'm%')". An attacker could then retrieve
the admin password quite quickly.

IMHO this is an example of what should never be done.

// very bad (especially in PHP where you never know the type of your
variables)
sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " + orderId;

// slightly better (and safe)
sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " + int( orderId );

// correct (PHP syntax)
pg_query_params( "SELECT * FROM ORDERS WHERE ORDER_ID = $1",
array( orderId ))
db_query( "SELECT * FROM ORDERS WHERE ORDER_ID = %s", array( orderId ))

// correct (Python syntax)
cursor.execute( "SELECT * FROM ORDERS WHERE ORDER_ID = %s", ( orderId, ))

The last two don't complain if orderId is a string, it will be correctly
quoted, and then postgres will complain only if it is a string which does
not contain a number. This is useful in PHP where you never know what type
you actually have.

The little function in my previous mail is also useful for mysql which has
no support for parameterized queries.

"Thomas Mueller" <thomas.tom.mueller@gmail.com> writes:

Also, programming languages such as Java don't support tainting. And it's
again in the hand of the developer to use it, not use it, or use it in the
wrong way. There should be a way for an admin to enforce using it, and using
it correctly.

I bet you could do something clever with Java.

Something like making the Execute() stmt take a special kind of string object
which enforces that it can only be constructed as static final and takes a
String as a constructor argument . That would let you use literals in the
queries but bar you from including any user input at runtime. You could even
include some methods for assembling such StaticStrings in useful ways which
would let you build queries dynamically out of immutable pieces.

I think you're tilting at windmills if you want to bar *all* literals. That's
just too big of a usability hit and as you pointed out with the common use
case of dynamically choosing ORDER BY it doesn't even catch other common
cases. You need to step back and find a way to prevent user input from ending
up in the query regardless of whether it's in a literal or not.

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

On Tue, Apr 29, 2008 at 12:25 AM, Thomas Mueller
<thomas.tom.mueller@gmail.com> wrote:

What do you think about it? Do you think it makes sense to implement
this security feature in PostgreSQL as well? If not why not? Does
PostgreSQL have another solution or plan to solve the SQL injection
problem?

Damn, am I the only person who likes the idea?

To those suggesting that it's just treating the symptom: well of
course it is. But using e.g. Exec-Shield / PIE / stack protection
weren't bad ideas just because buffer overflows are the fault of the
application developer. And who wants to grep through every module they
install on their system every time they do an update just in case some
feature that they never use has added a bad query? Assuming they have
the source. PHP apps are notorious for it, of course, but it isn't
just them.

Now, I reckon the only way to sanely do it without mucking up people's
ad-hoc queries would be to have it as a permission that would default
to on, but be REVOKE-able. Then it can be revoked from the user/role
that $untrusted application connects as, but still allow people to get
in from a trusted account and get their hands dirty when they need to.

Would it catch ALL holes? No, as we saw in the order by case, and
there are probably others (not sure if I like the proposed solution
for that, btw). Would it catch a fair number? Absolutely.

Cheers

Tom

Hi,

In C the best practice is to use #define for constants. In C++ you
have 'const', in Java 'static final'. Unfortunately the 'named
constant' concept doesn't exist in SQL. I think that's a mistake. I
suggest to support CREATE CONSTANT ... VALUE ... and DROP CONSTANT
..., example: CREATE CONSTANT STATE_ACTIVE VALUE 'active'.

of course you mean:

CREATE CONSTANT state_active TEXT VALUE 'active'; ? ;)

interesting idea, would that mean PG complaints on queries

SELECT state_active FROM sometable ... because
state_active is already defined as constant?

What about local session variables? Usefull as well...

I think this is really a big effort :-)

Greets
Tino

"Tom Dunstan" <pgsql@tomd.cc> writes:

Damn, am I the only person who likes the idea?

Just about. The reason that this idea isn't going anywhere is that its
cost/benefit ratio is untenably bad. Forbidding literals will break
absolutely every SQL-using application on the planet, and in many cases
fixing one's app to obey the rule would be quite painful (consider
especially complex multi-layered apps such as are common in the Java
world). In exchange for that, you get SQL injection protection that
has got a lot of holes in it, plus it stops protecting you at all
unless you are using a not-SQL-standard database. That tradeoff is
not happening, at least not in any nontrivial application.

Analogies such as PIE just point up the difference: for 99% of
applications, you can enable PIE without doing any more work than
adding a compile switch. If people were looking at major surgery
on most of their apps to enable it, the idea would never have gone
anywhere.

If you're going to ask people to do significant revision of their
apps to gain security, they're going to want it to work no matter
what database they run their apps against. This is why you need
a client-side solution such as tainting.

regards, tom lane

Tino Wildenhain wrote:

Hi,

In C the best practice is to use #define for constants. In C++ you
have 'const', in Java 'static final'. Unfortunately the 'named
constant' concept doesn't exist in SQL. I think that's a mistake. I
suggest to support CREATE CONSTANT ... VALUE ... and DROP CONSTANT
..., example: CREATE CONSTANT STATE_ACTIVE VALUE 'active'.

of course you mean:

CREATE CONSTANT state_active TEXT VALUE 'active'; ? ;)

Why does he mean that? Manifest constants are not typed in plenty of
languages.

interesting idea, would that mean PG complaints on queries

SELECT state_active FROM sometable ... because
state_active is already defined as constant?

Right, this would be a major can of worms. The only way it could work, I
suspect, is by segregating the identifier space to remove ambiguity
between constants and other identifiers.

cheers

andrew

* Tom Lane <tgl@sss.pgh.pa.us> [080429 10:59]:

"Tom Dunstan" <pgsql@tomd.cc> writes:

Damn, am I the only person who likes the idea?

Just about. The reason that this idea isn't going anywhere is that its
cost/benefit ratio is untenably bad. Forbidding literals will break
absolutely every SQL-using application on the planet, and in many cases
fixing one's app to obey the rule would be quite painful (consider
especially complex multi-layered apps such as are common in the Java
world). In exchange for that, you get SQL injection protection that
has got a lot of holes in it, plus it stops protecting you at all
unless you are using a not-SQL-standard database. That tradeoff is
not happening, at least not in any nontrivial application.

Analogies such as PIE just point up the difference: for 99% of
applications, you can enable PIE without doing any more work than
adding a compile switch. If people were looking at major surgery
on most of their apps to enable it, the idea would never have gone
anywhere.

I guess my database apps qualify as "nontrivial". I'm pretty sure that
I *could* enable something like this in all my web-facing apps *and* my
compiled C/C++ apps and not have any troubles.

And I happen to have programs/code that fail on PIE/execshield stuff.

I guess everything is relative.

That said, though *I* like the idea (and since I develop against
PostgreSQL 1st and use params for my queries I would consider it a nice
tool to "keep me honest"), I can easily see that the cost/benefit ratio
on this could be quite low and make it not worth the code/support
necessary.

If you're going to ask people to do significant revision of their
apps to gain security, they're going to want it to work no matter
what database they run their apps against. This is why you need
a client-side solution such as tainting.

Well, just because a tool is available doesn't mean people have to use
it. I mean, we have PostgreSQL, and we think that's worth it, even
though to use it, "everybody" has to do significant revision of their
apps.

a.

--
Aidan Van Dyk Create like a god,
aidan@highrise.ca command like a king,
http://www.highrise.ca/ work like a slave.

"Aidan Van Dyk" <aidan@highrise.ca> writes:

That said, though *I* like the idea (and since I develop against
PostgreSQL 1st and use params for my queries I would consider it a nice
tool to "keep me honest"), I can easily see that the cost/benefit ratio
on this could be quite low and make it not worth the code/support
necessary.

Note that using parameters even for things which are actually constants is not
really very desirable. If you have a query like:

SELECT * FROM users WHERE userid = ? AND status = 'active'

a) It makes things a lot clearer to when you call Execute($userid) which
values are actually the key user-provided data. In more complex queries it
can be quite confusing to have lots of parameters especially if the query
itself only makes sense if you know what values will be passed.

b) It allows the database to take advantage of statistics on "status" that
might not otherwise be possible.

Parameters are definitely the way to go for dynamic user data but for
constants which are actually an integral part of the query and not parameters
you're passing different values for each time it's actually clearer to include
them directly in the query.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's On-Demand Production Tuning

If you're going to ask people to do significant revision of their
apps to gain security, they're going to want it to work no matter
what database they run their apps against.  This is why you need
a client-side solution such as tainting.

Or if people are going to re-write their applications anyway, we'd want at
least a theoretically robust and flexible approach like libdejector, which
lets you identify which parts of a query structure are modifiable and
which are not.

For example, some applications need to replace whole phrases:

$criteria = "WHERE $var1 = '$var2'"

This is a very common approach for dynamic search screens, and really not
covered by placeholder approaches.

--
--Josh

Josh Berkus
PostgreSQL @ Sun
San Francisco

For example, some applications need to replace whole phrases:

$criteria = "WHERE $var1 = '$var2'"

This is a very common approach for dynamic search screens, and really not
covered by placeholder approaches.

Python, again :

params = {
'column1': 10,
'column2': "a st'ring",
}

where = " AND ".join( "%s=%%s" % (key,value) for key,value in
params.items() )
cursor.execute( "SELECT * FROM table WHERE " + where, params )

I use the same approach (albeit more complicated) in PHP.

For complex expressions you can play with arrays etc, it is not that
difficult.
Or you just do :

$criteria = db_quote_query( "WHERE $var1 = %s", array( $var2 ))

using the function I posted earlier.

This supposes of course that $var1 which is the column name, comes from a
known source, and not user input.
In that case, $var1 will probably be the form field name, which means it
is specified by the programmer a few lines prior in the code.

Hi Martijn,

The problem is not only quotes. The problem is all kinds of user
input. For example: sql = "SELECT * FROM ORDERS WHERE ORDER_ID = " +
orderId; This is not a problem if orderId is a number. But what if
it's a String? For example "1 AND (SELECT * FROM USERS WHERE
NAME='admin' AND PASSWORD LIKE 'm%')". An attacker could then retrieve
the admin password quite quickly.

In other words, your programmer was stupid. And your example doesn't
work because no matter what the string is it can't return anything
other than rows from the orders table. If you're worried about them
using semicolons to introduce another query, prepare has prohibited
that for a long time already.

The attack goes as follows: WHERE ORDER_ID = 1 yields 1 rows. WHERE
ORDER_ID = 1 AND (SELECT * FROM USERS WHERE NAME='admin' AND PASSWORD
LIKE 'a%') yields 0 rows. OK that means that the admin password
doesn't start with an 'a'. If WHERE ORDER_ID = 1 AND (SELECT * FROM
USERS WHERE NAME='admin' AND PASSWORD LIKE 'b%') yields 1 row we know
the admin password starts with 'b'. For an average password length of
6 it takes 6 * 64 queries to get the password, plus some to get the
user name, plus maybe a few to get the table name and column name
correct.

But as far as I'm concerned, the real killer is that it would make
using any interactive query interface impossible.

No. Literals is an access right, and the interactive query tool may
have that access right. Let's say we have a APP_ROLE (for the
application itself) and a QUERY_ROLE. The default is literals are
enabled, that means the query tool can use literals. For the
application, the administrator may chooses to revoke the right to use
text and number literals using REVOKE LITERAL_TEXT, LITERAL_NUMBER
FROM APP_ROLE. Or the developer himself may want to try out if his
application is safe, and temporarily disables LITERAL_TEXT first. He
then runs the test cases and fixes the problems. Afterwards, he may
disable even LITERAL_NUMBER and try again. For production, maybe
literals are enabled.

Regards,
Thomas