Обсуждение: Protection from SQL injection
Hi, 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: SET ALLOW_LITERALS NONE; Afterwards, SQL statements with text are not allowed any more for this session. That means, SQL statement of the form "SELECT * FROM USERS WHERE PASSWORD='qerkllkj'" will fail with the exception 'Literals are not allowed, please use parameters'. It is like the database does not know what ='qerkllkj' means. Only statements of the secure form are allowed, for example "SELECT * FROM USERS WHERE PASSWORD=?". This solves the problem because SQL injection is almost impossible if user input is not directly embedded in SQL statements. The 'ALLOW_LITERALS NONE' mode is enabled by the developer itself, or by an administrator. It is still possible to generate SQL statements dynamically, and use the same APIs as before, as long as SQL statements don't include literals. Literals can still be used when using query tools, or in applications considered 'safe'. To ease converting the application to use parameterized queries, there should be a second mode where number literals are allowed: SET ALLOW_LITERALS NUMBERS. To allow all literals, execute SET ALLOW_LITERALS ALL (this is the default setting). So far this feature is implemented in my little database H2. More information about this feature is described here: http://www.h2database.com/html/advanced.html#sql_injection I know about the Perl taint mode, but this is only for Perl. I also know about disabling multi-statement commands (only solves part of the problem). PostgreSQL should also support database level 'constants' that are similar to constants in other programming languages, otherwise application level constants (such as 'active') can't be used in queries directly (I propose to add new SQL statements CREATE CONSTANT ... VALUE ... and DROP CONSTANT ..., example: CREATE CONSTANT STATE_ACTIVE VALUE 'active'). I also know the 'disabling literals' feature does not solve SQL injection completely: for example 'ORDER BY injection' where an application dynamically adds the column to sort on based on a hidden 'sort column' field in a web app. To solve that I suggest to support parameterized ORDER BY: ORDER BY ? where ? is an integer. Then, instead of using SET ALLOW_LITERALS NONE the use of literals should probably be two access right (REVOKE LITERAL_TEXT, LITERAL_NUMBER FROM APP_ROLE). Those are details that still need to be discussed. 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? Regards, Thomas P.S. I have send this proposal to pgsql-sql@postgresql.org first and got replies, but I would like to get some feedback from the PostgreSQL developers as well.
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 rawif( $params === false ) return $sql;if( !is_array( $params )) $params = array(
$params);
// quote paramsforeach( $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 queryif( 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;
}
>
> SET ALLOW_LITERALS NONE;
>
> Afterwards, SQL statements with text are not allowed any more for this
> session. That means, SQL statement of the form "SELECT * FROM USERS
> WHERE PASSWORD='qerkllkj'" will fail with the exception 'Literals are
> not allowed, please use parameters'. It is like the database does not
> know what ='qerkllkj' means. Only statements of the secure form are
> allowed, for example "SELECT * FROM USERS WHERE PASSWORD=?". This
> solves the problem because SQL injection is almost impossible if user
> input is not directly embedded in SQL statements.
>
> The 'ALLOW_LITERALS NONE' mode is enabled by the developer itself, or
> by an administrator. It is still possible to generate SQL statements
> dynamically, and use the same APIs as before, as long as SQL
> statements don't include literals. Literals can still be used when
> using query tools, or in applications considered 'safe'. To ease
> converting the application to use parameterized queries, there should
> be a second mode where number literals are allowed: SET ALLOW_LITERALS
> NUMBERS. To allow all literals, execute SET ALLOW_LITERALS ALL (this
> is the default setting).
>
> So far this feature is implemented in my little database H2. More
> information about this feature is described here:
> http://www.h2database.com/html/advanced.html#sql_injection
>
> I know about the Perl taint mode, but this is only for Perl. I also
> know about disabling multi-statement commands (only solves part of the
> problem). PostgreSQL should also support database level 'constants'
> that are similar to constants in other programming languages,
> otherwise application level constants (such as 'active') can't be used
> in queries directly (I propose to add new SQL statements CREATE
> CONSTANT ... VALUE ... and DROP CONSTANT ..., example: CREATE CONSTANT
> STATE_ACTIVE VALUE 'active'). I also know the 'disabling literals'
> feature does not solve SQL injection completely: for example 'ORDER BY
> injection' where an application dynamically adds the column to sort on
> based on a hidden 'sort column' field in a web app. To solve that I
> suggest to support parameterized ORDER BY: ORDER BY ? where ? is an
> integer. Then, instead of using SET ALLOW_LITERALS NONE the use of
> literals should probably be two access right (REVOKE LITERAL_TEXT,
> LITERAL_NUMBER FROM APP_ROLE). Those are details that still need to be
> discussed.
>
> 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?
>
> Regards,
> Thomas
>
> P.S. I have send this proposal to pgsql-sql@postgresql.org first and
> got replies, but I would like to get some feedback from the PostgreSQL
> developers as well.
>
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/ > Please line up in a tree and maintain the heap invariant while > boarding. Thank you for flying nlogn airlines.
>> 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 makessense 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
* Gregory Stark <stark@enterprisedb.com> [080429 14:20]: > "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. These are all things to consider. I haven't (yet) needed a dynamic query like that in my published apps because I would have a prepared statement for the various status options, and my choice was to have a couple prepared statements around instead of having a dynamic statement thats re-planned on every query. Most of my published applications *are* simple, and I tend to consolidate as much of my "business logic" in the database as possible and a "known" set of queries shared by all the related apps, relying heavily on view, triggers, and functions, so the queries in my web-side and C-side applications really are very simple and straight forward. I purposely choose to have "simple static queries" in my apps. So a mode which "rejects" queries with literals/constants in them would catch "bugs" in my code. Those "bugs" really could be cosmetic, and still "valid SQL" queries, but one of them could be a valid one which could be an injection vector. And so far the statistic/plan selection problems haven't made any of my queries yet become performance problems... Again, everything is relative. a. -- Aidan Van Dyk Create like a god, aidan@highrise.ca command like a king, http://www.highrise.ca/ work like a slave.
Hi,
For PostgreSQL the 'disable literals' feature would be great
publicity: PostgreSQL would be the first only major database that has
a good story regarding SQL injection. Yes it's not the magic silver
bullet, but databases like MS SQL Server, Oracle or MySQL would look
really bad.
> [literals...] a permission that would default to on, but be REVOKE-able.
Exactly.
> Forbidding literals will break absolutely every SQL-using application on the planet
Well, it's optional. If a developer or admin wants to use it, he will
know that it could mean some work. Even if the feature is not enabled,
it's still good to have it. And using constants will help document the
application.
> CREATE CONSTANT state_active TEXT VALUE 'active'; ? ;)
Not necessarily. The database knows that 'active' is a text, no need
to repeat that. Auto-detecting data types already works: CREATE TABLE
TEST AS SELECT 1 AS ID FROM DUAL will result in an int4. That's enough
for constants. But I don't mind using explicit data types.
> Note that using parameters even for things which are actually constants is not really very desirable. If you have a
querylike: SELECT * FROM users WHERE userid = ? AND status = 'active'
Using 'active' anyway is bad: Think about typos. The constant concept
(that exists in every language except SQL) would be good in any case:
SELECT * FROM users WHERE userid = ? AND status = STATUS_ACTIVE (or
CONST.STATUS_ACTIVE if it's in the CONST schema).
> libdejector
It's a good tool, but it's more work for the developer than disabling
literals (because for each query you need to add a exemplar).
> dynamic search screens
> $criteria = "WHERE $var1 = '$var2'"
In Java (sorry about that ;-) I would write:
PreparedStatement prep = conn.prepareStatement("SELECT * FROM ITEMS
WHERE " + var1 + " = ?");
prep.setString(1, var2);
Regards,
Thomas
Thomas Mueller wrote: >> Forbidding literals will break absolutely every SQL-using application on the planet >> > > Well, it's optional. If a developer or admin wants to use it, he will > know that it could mean some work. Even if the feature is not enabled, > it's still good to have it. And using constants will help document the > application. > > > What is not optional is the probably maintenance complexity of this scheme. Moreover, it seems unlikely that it will even cover the field. A partial cloak might indeed be worse than none, in that it will give some developers an illusion of having security. Before we embarked on such an enterprise, I would personally want to see fairly loud clamor from our user base for it. cheers andrew
On Tue, Apr 29, 2008 at 04:33:01PM -0400, Andrew Dunstan wrote: > Moreover, it seems unlikely that it will even cover the field. A partial > cloak might indeed be worse than none, in that it will give some developers > an illusion of having security. I think this is a really important point, and one that isn't getting enough attention in this discussion. Half a security measure is almost always worse than none at all, exactly because people stop thinking they have to worry about that area of security at all. I think without a convincing argument that the proposal will even come close to covering most SQL injection cases, it's a bad idea. A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
Thomas, > For PostgreSQL the 'disable literals' feature would be great > publicity: PostgreSQL would be the first only major database that has > a good story regarding SQL injection. Yes it's not the magic silver > bullet, but databases like MS SQL Server, Oracle or MySQL would look > really bad. Please don't let the debate over this break your enthusiasm for improving PostgreSQL security. We really care about security, which is why we want to run your proposal throught the gauntlet. You said you've done this for H2. Isn't H2 only accessable through Java, though? How many people are using literals in Java? And, as of this week MSSQL already looks really bad. 300,000 worm-infected servers, and counting! -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
[I know, I know, bad form] On Tue, Apr 29, 2008 at 04:55:21PM -0400, Andrew Sullivan wrote: > thinking they have to worry about that area of security at all. I > think without a convincing argument that the proposal will even come > close to covering most SQL injection cases, it's a bad idea. To be perfectly clear, I also think that the reverse is true: if a fairly complete design was demonstrated to be possible such that it covered just about every case, I'd be all for it. (I sort of like the suggestion up-thread, myself, which is to have a GUC that disables multi-statement commands. That'd probably cover a huge number of cases, and combined with some sensible quoting rules in client libraries, would quite possibly be enough.) A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
> (I sort of like the > suggestion up-thread, myself, which is to have a GUC that disables > multi-statement commands. That'd probably cover a huge number of > cases, and combined with some sensible quoting rules in client > libraries, would quite possibly be enough.) MySQL did this already. -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco
On Tue, 29 Apr 2008 22:18:48 +0200 Thomas Mueller wrote: > For PostgreSQL the 'disable literals' feature would be great > publicity: PostgreSQL would be the first only major database that has > a good story regarding SQL injection. Yes it's not the magic silver > bullet, but databases like MS SQL Server, Oracle or MySQL would look > really bad. I don't think so. Given the fact that enabling this feature by default would break almost all applications, you have to disable this by default. No use here because almost nobody will know about it. Oh, and i can see the headlines: "New PostgreSQL feature breaks 99% applications". > > Forbidding literals will break absolutely every SQL-using application on the planet > > Well, it's optional. If a developer or admin wants to use it, he will > know that it could mean some work. The developers and admins who know about this feature and want to use it are also the developers and admins who know about SQL injections. Eventually the code quality produced by this ppl is higher than average and less likely to have such basic faults. > Even if the feature is not enabled, it's still good to have it. Huh? How this? Just because one can say "We have a feature against SQL injections" which will not be used by literally anyone? Kind regards -- Andreas 'ads' Scherbaum German PostgreSQL User Group
On Wed, Apr 30, 2008 at 1:48 AM, Thomas Mueller <<a
href="mailto:thomas.tom.mueller@gmail.com">thomas.tom.mueller@gmail.com</a>>wrote:<br /><div
class="gmail_quote"><blockquoteclass="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt
0pt0.8ex; padding-left: 1ex;"> Hi,<br /><br /> For PostgreSQL the 'disable literals' feature would be great<br />
publicity:<br/><br /></blockquote></div><br />'publicity' is something this community does not crave for, at least not
featurewise. If that were the case we would have had a million half-baked features in Postgres by now.<br clear="all"
/><br/>-- <br />gurjeet[.singh]@EnterpriseDB.com<br />singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com<br
/><br/>EnterpriseDB <a href="http://www.enterprisedb.com">http://www.enterprisedb.com</a><br /><br />Mail sent from my
BlackLaptopdevice
"Josh Berkus" <josh@agliodbs.com> writes: >> (I sort of like the >> suggestion up-thread, myself, which is to have a GUC that disables >> multi-statement commands. That'd probably cover a huge number of >> cases, and combined with some sensible quoting rules in client >> libraries, would quite possibly be enough.) > > MySQL did this already. Did you guys miss Tom's comment up-thread? Postgres already does this if you use PQExecParams(). -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Greg, > Did you guys miss Tom's comment up-thread? Postgres already does this if > you use PQExecParams(). Keen. Now we just need to get the driver developers to implement it. I imagine Java does. -- Josh Berkus PostgreSQL @ Sun San Francisco
On Tue, 2008-04-29 at 16:01 -0400, Aidan Van Dyk wrote: > Most of my published applications *are* simple, and I tend to > consolidate as much of my "business logic" in the database as possible > and a "known" set of queries shared by all the related apps, relying > heavily on view, triggers, and functions, so the queries in my web-side > and C-side applications really are very simple and straight forward. I a company I worked, we got ( almost ? ) the same result by doing all access using functions and REVOKE-ing frontend app users all privileges on anything else. So almost all sql issued by apps looks like "SELECT * FROM some_func(p1, p2, ..., pn)" This has a lot of nice properties, among others ability to do lots of database code fixing on live 27/4 apps without frontends never noticing. > I purposely choose to have "simple static queries" in my apps. So a > mode which "rejects" queries with literals/constants in them would catch > "bugs" in my code. Hmm - maybe a mode where functions accept only parameters would be needed for enforcing this on current server code. Anyway, with pl/proxy partitioning/loadbalancing running on data-empty servers, code injection would be quite hard even without params-only mode. > Those "bugs" really could be cosmetic, and still > "valid SQL" queries, but one of them could be a valid one which could be > an injection vector. Could we also get a mode, where PREPARE would only be allowed for queries of the form "SELECT * FROM func(?,?,?,?,?); :) --------------------- Hannu
On Tue, 29 Apr 2008, Josh Berkus wrote: >> Did you guys miss Tom's comment up-thread? Postgres already does this if >> you use PQExecParams(). > > Keen. Now we just need to get the driver developers to implement it. I > imagine Java does. > The JDBC driver takes a multi-command statement and splits it up to be able to use the extended query protocol. So the JDBC driver is actually doing the reverse of your suggestion. For us it was a decision to ease the transition from V2 to V3 protocol and not break code that used to work. Kris Jurka
On Tue, Apr 29, 2008 at 09:02:30PM -0400, Gregory Stark wrote: > Did you guys miss Tom's comment up-thread? Postgres already does this if you > use PQExecParams(). I did, yes. Thanks for the clue. OTOH, I do see the OP's point that it'd be nice if the DBA could enforce this rule. Maybe a way of insisting on PQExecParams() instead of anything else? A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
Hi, > How many people are using literals in Java? Not sure if I understood the question... In Java most people use constants (final static). 'Checkstyle' can find 'magic numbers' in the source code. If the constants feature was very important in SQL, people would have requested it, and it would be in the SQL standard by now. There is a workaround: user defined functions. > Disabling multi-statement commands Disabling multi-statement commands just limits the effect of SQL injection. Disabling literals actually protects from SQL injection. Both features are important. > ( almost ? ) the same result by doing all access using functions This also doesn't protect from SQL injection, it only limits the effect. > Half a security measure is almost always worse than none at all Cars and houses have locks. Locks can't fully protect you. Do they give the illusion security? Maybe. But it's definitely better to have them. > headlines: "New PostgreSQL feature breaks 99% applications" Not if it's disabled by default. What about "New PostgreSQL feature offers 95% protection from SQL injection"? > The developers and admins who know about this feature and want to use it... > quality produced by this ppl is higher than average and less likely to have such basic faults. Maybe. I found some problems in my code when enabling this feature, and I thought I was save (or paranoid :-). Regards, Thomas
Andrew Dunstan wrote: > > > 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. Well but in this case we want them to prevent easy sql injection and therefore arbitrary macro expansion like in those "plenty of languages" does not seem like a good idea to me. Cheers Tino
> Could we also get a mode, where PREPARE would only be allowed for > queries of the form "SELECT * FROM func(?,?,?,?,?); :) Actually, that is similar to the concept of "global prepared statements" that I proposed some time ago, but I will not have time to write the patch, alas...Idea was that the DBA can create a list of SQL statements (with privileges about who can execute them, just like functions) which are prepared on-demand at the first EXECUTE by the client.This would enhance performance (but for performance I like the ideaof caching plans better).It would be pretty cumbersome, though, to execute dynamic SQL like the typical search query...
Hi, Constants are just convenience: instead of constants, user defined functions can be used. This already works, however it's a bit verbose: CREATE FUNCTION STATE_ACTIVE() RETURNS VARCHAR AS $$ BEGIN RETURN 'active'; END; $$ LANGUAGE PLPGSQL; Usage is almost the same: SELECT * FROM USERS WHERE STATE=STATE_ACTIVE(); > therefore arbitrary macro expansion like in those "plenty of languages" > does not seem like a good idea to me. This is _not_ macro expansion as in C '#define'. Constants are typed, as in C++ 'const' and Java 'static final'. The question is only: should the user explicitly state the data type, or should the data type be deduced from the value. Both is possible: CREATE CONSTANT STATE_ACTIVE VALUE 'active'; CREATE CONSTANT STATE_ACTIVE TEXT VALUE 'active'; Regards, Thomas
On Wed, Apr 30, 2008 at 8:52 PM, Thomas Mueller <thomas.tom.mueller@gmail.com> wrote:
Maybe we can extend the SQL's WITH clause do declare the constant along with the query, and not separate from the query.
WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
and let postgres allow literals only in the WITH clause.
Also, IMHO, the type of the expression should be automatically deduced. The right hand side should be an expression and not just a string or numeric literal. For eg. the above query can be written as:
WITH
CONSTANT c_jobrole = 'clerk',
CONSTANT c_deptname = 'FINANCE'::text,
CONSTANT c_dept = (SELECT dname FROM dept WHERE dname = c_deptname)
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
so the expression can be CAST'd into appropriate type wherever needed.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
Mail sent from my BlackLaptop device
Hi,
Constants are just convenience: instead of constants, user defined
functions can be used. This already works, however it's a bit verbose:
CREATE FUNCTION STATE_ACTIVE() RETURNS VARCHAR AS
$$ BEGIN RETURN 'active'; END; $$ LANGUAGE PLPGSQL;
Usage is almost the same:
SELECT * FROM USERS WHERE STATE=STATE_ACTIVE();This is _not_ macro expansion as in C '#define'. Constants are typed,
> therefore arbitrary macro expansion like in those "plenty of languages"
> does not seem like a good idea to me.
as in C++ 'const' and Java 'static final'. The question is only:
should the user explicitly state the data type, or should the data
type be deduced from the value. Both is possible:
CREATE CONSTANT STATE_ACTIVE VALUE 'active';
CREATE CONSTANT STATE_ACTIVE TEXT VALUE 'active';
Maybe we can extend the SQL's WITH clause do declare the constant along with the query, and not separate from the query.
WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
and let postgres allow literals only in the WITH clause.
Also, IMHO, the type of the expression should be automatically deduced. The right hand side should be an expression and not just a string or numeric literal. For eg. the above query can be written as:
WITH
CONSTANT c_jobrole = 'clerk',
CONSTANT c_deptname = 'FINANCE'::text,
CONSTANT c_dept = (SELECT dname FROM dept WHERE dname = c_deptname)
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
so the expression can be CAST'd into appropriate type wherever needed.
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
Mail sent from my BlackLaptop device
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> Maybe we can extend the SQL's WITH clause do declare the constant along with
> the query, and not separate from the query.
> WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10
> SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
[ scratches head... ] And that will provide SQL injection protection how?
Anyway, you hardly need new syntax to do that, I'd expect
WITH SELECT 'clerk' AS c_jobrole ...
to accomplish it just fine.
regards, tom lane
On Wed, Apr 30, 2008 at 10:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Well, if the the query was:
WITH CONSTANT c_jobrole = <value from a FORM text field>, CONSTANT c_dept = 10
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
And if the attack supplied a value 'clerk OR 1=1' the final query (after replacing constants) would look like this:
SELECT * FROM emp WHERE jobrole = 'clerk OR 1=1' and deptno = 10;
The attacker was not able to inject any new code there.
(reiterates: and let postgres allow literals only in the WITH clause)
I am not sure I understood this example.
Best regards,
--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
Mail sent from my BlackLaptop device
"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:[ scratches head... ] And that will provide SQL injection protection how?
> Maybe we can extend the SQL's WITH clause do declare the constant along with
> the query, and not separate from the query.
> WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10
> SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
Well, if the the query was:
WITH CONSTANT c_jobrole = <value from a FORM text field>, CONSTANT c_dept = 10
SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept;
And if the attack supplied a value 'clerk OR 1=1' the final query (after replacing constants) would look like this:
SELECT * FROM emp WHERE jobrole = 'clerk OR 1=1' and deptno = 10;
The attacker was not able to inject any new code there.
(reiterates: and let postgres allow literals only in the WITH clause)
Anyway, you hardly need new syntax to do that, I'd expect
WITH SELECT 'clerk' AS c_jobrole ...
to accomplish it just fine.
I am not sure I understood this example.
Best regards,
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | indiatimes | yahoo }.com
EnterpriseDB http://www.enterprisedb.com
Mail sent from my BlackLaptop device
* Gurjeet Singh <singh.gurjeet@gmail.com> [080430 13:38]: > Well, if the the query was: > > WITH CONSTANT c_jobrole = <value from a FORM text field>, CONSTANT c_dept = > 10 > SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; > > And if the attack supplied a value 'clerk OR 1=1' the final query (after > replacing constants) would look like this: > > SELECT * FROM emp WHERE jobrole = 'clerk OR 1=1' and deptno = 10; > > The attacker was not able to inject any new code there. > > (reiterates: and let postgres allow literals only in the WITH clause) Sure, but you've only changed the attack vector: set <value from a form text field> to be:something' SELECT 1; DELETE FROM users; WITH c_jobrole = '1 And you get:WITH CONSTANT cjobrole = 'someting' SELECT 1; DELETE FROM users; WITHc_jobrole = '1' SELECT * FROM emp WHEREjobrole = c_jobrole; Sure, if the <value from form> is properly escaped and sanitized, it wouldn't be a problem. But if you are sure that all data is properly escaped and sanitized, then the whole "don't allow literals" extra protection isn't needed either, and you don't need to go looking for ways to avoid the literals in queries. -- Aidan Van Dyk Create like a god, aidan@highrise.ca command like a king, http://www.highrise.ca/ work like a slave.
On Wed, Apr 30, 2008 at 10:20:09AM -0400, Andrew Sullivan wrote:
> On Tue, Apr 29, 2008 at 09:02:30PM -0400, Gregory Stark wrote:
> > Did you guys miss Tom's comment up-thread? Postgres already does this if you
> > use PQExecParams().
>
> I did, yes. Thanks for the clue. OTOH, I do see the OP's point that
> it'd be nice if the DBA could enforce this rule. Maybe a way of
> insisting on PQExecParams() instead of anything else?
Create a function somewhere:
void PQexec()
{ die();
}
And it will override the one in the shared library. In other languages
subclassing should be able to provide the same effect.
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.
Martijn van Oosterhout <kleptog@svana.org> writes:
> void PQexec()
> {
> die();
> }
Actually disabling PQexec seems like a pretty poor decision; it breaks
working code to no purpose. Also, doing it on the client side means
you're at risk of some clients being secure and some not. I thought
what we were discussing was a server-side GUC parameter that would
disallow more than one SQL statement per PQexec.
regards, tom lane
On Wed, Apr 30, 2008 at 05:33:38PM -0400, Tom Lane wrote: > you're at risk of some clients being secure and some not. I thought > what we were discussing was a server-side GUC parameter that would > disallow more than one SQL statement per PQexec. That was certainly what I was intending, yes. The _principal_ trick with SQL injection is to fool the application into somehow handing a ";" followed by an arbitrary SQL statement. There are of course other things one can do, but most of them are constrained to abuse of statements your application already performs. This injection problem, on the other hand, allows an attacker to do whatever they want. Obviously, if the server simply throws an error whenever one tries to do this, the attack will be foiled. It sounded to me like a patch that implemented this was already rejected. I agree that it's a bit filthy, and I'd way prefer that people build their applications such that these vectors aren't open in the first place. But given the prevalence of quick and dirty development with code one hasn't always completely vetted, this might be a nice feature in some environments. As long as it's possible to turn it off (we'd probably need to make it require a server restart to make it really effective), I think it could be useful. A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
"Andrew Sullivan" <ajs@commandprompt.com> writes: > The _principal_ trick with SQL injection is to fool the application > into somehow handing a ";" followed by an arbitrary SQL statement. > There are of course other things one can do, but most of them are > constrained to abuse of statements your application already performs. > This injection problem, on the other hand, allows an attacker to do > whatever they want. They're the principal trick only because they're the most convenient. If you block them (as you can today by using PQExecParams() !!!) then people will switch to other things. c.f. http://www.areino.com/hackeando/ (there is a semicolon here but that's a microsoft-ism, postgres would actually be more affected by this style of attack without the semicolon) -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support!
Gregory Stark <stark@enterprisedb.com> writes:
> "Andrew Sullivan" <ajs@commandprompt.com> writes:
>> The _principal_ trick with SQL injection is to fool the application
>> into somehow handing a ";" followed by an arbitrary SQL statement.
> They're the principal trick only because they're the most convenient. If you
> block them (as you can today by using PQExecParams() !!!) then people will
> switch to other things.
Sure, modifying the WHERE clause is still possible, but the attacker is
a lot more limited in what he can do if he can't tack on a whole new
command.
The important aspects of this that I see are:
1. Inexpensive to implement;
2. Unlikely to break most applications;
3. Closes off a fairly large class of injection attacks.
The cost/benefit ratio looks pretty good (unlike the idea that started
this thread...)
regards, tom lane
On Thu, May 01, 2008 at 11:26:21AM -0400, Tom Lane wrote: > > 1. Inexpensive to implement; > 2. Unlikely to break most applications; > 3. Closes off a fairly large class of injection attacks. > > The cost/benefit ratio looks pretty good (unlike the idea that started > this thread...) That's a much more elegant way of putting what I thought. Thanks, Tom. A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
> Sure, modifying the WHERE clause is still possible, but the attacker is > a lot more limited in what he can do if he can't tack on a whole new > command. I hacked into a site like that some day to show a guy that you shouldn't trust magicquotes (especially when you switch hosting providers and it's not installed at your new provider, lol).Binary search on the password field by adding some stuff to the WHERE...You couldstill wipe out tables (just add a "' OR 1;--" to the id in the url to delete somthing... But it's true that preventing multi-statements adds a layer of idiot-proofness... a rather thin layer... > > The important aspects of this that I see are: > > 1. Inexpensive to implement; > 2. Unlikely to break most applications; > 3. Closes off a fairly large class of injection attacks. > > The cost/benefit ratio looks pretty good (unlike the idea that started > this thread...) > > regards, tom lane >
Hi, > disallow more than one SQL statement per PQexec. I agree, it would help. > 1. Inexpensive to implement Disabling literals wouldn't be much harder to implement I believe, but I don't know the PostgreSQL internals. > 2. Unlikely to break most applications; That's true. > 3. Closes off a fairly large class of injection attacks. Unfortunately, it doesn't prevent SQL injection at all. Yes, updating and deleting data is harder (if the SQL injection is on a SELECT), but if an attacker only wants to destroy something he could use SETVAL. There is almost zero protection from reading data (no matter where the SQL injection is). It is quite simple to write an automated tool that downloads the whole database contents (first the catalog, and then all the data). Downloading the data would be a bit slower if the SQL injection is in the WHERE clause. Actually, it would be quite fun to write a generic tool ;-) Tools are usually used by the script kiddies. My website was once hacked by a 14 year old boy. He used a tool that read the admin password via SQL injection. Disabling literals is still the only way to actually protect from SQL injection. Except Meredith's libdejector, which is even a bit better as far as I see, but requires more work from the developer. I don't count Microsoft LINQ (or Java Quaere) currently because that would require a complete re-write of the application. Regards, Thomas
Thomas Mueller wrote: > Disabling literals is still the only way to actually protect from SQL > injection. Except Meredith's libdejector, which is even a bit better > as far as I see, but requires more work from the developer. I don't > count Microsoft LINQ (or Java Quaere) currently because that would > require a complete re-write of the application. > > > I honestly don't think there's any chance of this happening, for the many good reasons previously covered in this debate. cheers andrew
"Thomas Mueller" <thomas.tom.mueller@gmail.com> writes:
>> 1. Inexpensive to implement
> Disabling literals wouldn't be much harder to implement I believe, but
> I don't know the PostgreSQL internals.
You're ignoring the client-side costs of repairing broken applications.
(If it only broke applications that were in fact insecure, that would be
one thing, but having to change code that there is nothing wrong with
is not something that people will accept easily.)
> Disabling literals is still the only way to actually protect from SQL
> injection.
If it were actually a complete defense then maybe the costs would be
justifiable; but it isn't, as per previous discussion.
regards, tom lane
On Thu, May 01, 2008 at 06:33:07PM +0200, PFC wrote: > But it's true that preventing multi-statements adds a layer of > idiot-proofness... a rather thin layer... As I already said in a previous remark in this thread, I don't really like partial security solutions. What the "no multi-statement SQL" switch adds is a complete protection against _one class_ of injection attacks. What is nice about it is that it completely eliminates that class of attacks, so they are no longer something one needs to worry about. They do not, of course, prevent every kind of injection attack. I think the thread has already had ample evidence that such complete prevention is either impractical to implement, too costly to existing applications, too limiting, not actually effective (i.e. not really complete prevention), or some combination of the above. That's not an argument that the simple change that is effective for only one class of attacks is a bad idea. Making the battlefield smaller is one thing one can do to decrease one's exposure to attack. A -- Andrew Sullivan ajs@commandprompt.com +1 503 667 4564 x104 http://www.commandprompt.com/
**Andrew Sullivan wrote: > On Thu, May 01, 2008 at 06:33:07PM +0200, PFC wrote: > > > But it's true that preventing multi-statements adds a layer of > > idiot-proofness... a rather thin layer... > > As I already said in a previous remark in this thread, I don't really > like partial security solutions. > > What the "no multi-statement SQL" switch adds is a complete protection > against _one class_ of injection attacks. What is nice about it is > that it completely eliminates that class of attacks, so they are no > longer something one needs to worry about. > > They do not, of course, prevent every kind of injection attack. I > think the thread has already had ample evidence that such complete > prevention is either impractical to implement, too costly to existing > applications, too limiting, not actually effective (i.e. not really > complete prevention), or some combination of the above. > > That's not an argument that the simple change that is effective for > only one class of attacks is a bad idea. Making the battlefield > smaller is one thing one can do to decrease one's exposure to attack. Andrew, Tom, please just add this feature and let those who don't like it just send their complains to /dev/null. To be honest, I'm surprised it has not appeared sooner...I was thinking of hacking on postgres to implement it then I found this thread. I've already started looking at writing a proxy for postgres with the first important task to be converting multiple statements in the SQL TCP data stream to a series of spaces...for those that can't upgrade to a version where they can turn such a knob on. Also, I think the "no-multistatement SQL" should default to on. Why? Because interacting with the database is always through an action that you do and if you're being half way intelligent about it, you are always checking that each action succeeded before going on to the next. I think one of the more important exercises is to examine PHP and see if it bundles up any statements into more than one call to prepare or execute in perl. As with all security solutions, the end result is made up of many layers, there is rarely just one big gate. The more layers we have, regardless of how thick or thing they are, the better equipped we are to deal with attacks. Cheers, Darren
Darren Reed <darrenr+postgres@fastmail.net> writes:
> Also, I think the "no-multistatement SQL" should default to on.
Not happening, for backwards-compatibility reasons.
regards, tom lane
Darren Reed wrote:
> Because interacting with the database is always through an action
> that you do and if you're being half way intelligent about it, you
> are always checking that each action succeeded before going on to
> the next.
Hmm, it won't be pretty for the drivers that do PQexec("COMMIT; BEGIN").
The driver will think that it's in a transaction when in fact the second
command in the string has been ignored, and so it's not ...
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160
> Hmm, it won't be pretty for the drivers that do PQexec("COMMIT; BEGIN").
> The driver will think that it's in a transaction when in fact the second
> command in the string has been ignored, and so it's not ...
Any driver that is doing that should be shot.
- --
Greg Sabino Mullane greg@turnstep.com
PGP Key: 0x14964AC8 200805021325
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----
iEYEAREDAAYFAkgbTn4ACgkQvJuQZxSWSshKwwCfewZyRy/b6PvJrQn6pTlgsSDb
MeQAoM4sajlNKU17z3tVDqVTfqcyLf9N
=Fj0e
-----END PGP SIGNATURE-----
alvherre@commandprompt.com (Alvaro Herrera) writes:
> Darren Reed wrote:
>
>> Because interacting with the database is always through an action
>> that you do and if you're being half way intelligent about it, you
>> are always checking that each action succeeded before going on to
>> the next.
>
> Hmm, it won't be pretty for the drivers that do PQexec("COMMIT; BEGIN").
> The driver will think that it's in a transaction when in fact the second
> command in the string has been ignored, and so it's not ...
We have worked pretty hard around here to expunge use of drivers that
do this sort of thing. (Cough, cough, "dbrow"...)
Recent versions of PostgreSQL don't suffer too badly, but back in the
7.2/7.4 days, we had applications that left transactions open "<IDLE>
in transaction" for days at a time (if a user quit using the web app
without expressly logging out), with _atrocious_ results.
Andrew Sullivan recently had some choice words about the merits of
ENUM; I think the same applies to drivers that do
PQexec("COMMIT;BEGIN")...
--
output = ("cbbrowne" "@" "linuxfinances.info")
http://www3.sympatico.ca/cbbrowne/advocacy.html
:FATAL ERROR -- ILLEGAL ERROR
On Fri, May 02, 2008 at 03:58:01PM -0400, Chris Browne wrote:
> Andrew Sullivan recently had some choice words about the merits of
> ENUM; I think the same applies to drivers that do
> PQexec("COMMIT;BEGIN")...
Oh, heaven. I can at least think of ways to use ENUM such that you
can justify the trade-off. I can think of no excuse whatever for
PQexec("COMMIT; BEGIN"). That's just lazy and sloppy.
Note also that more recent releases, concurrent with the improvements
to the drivers, also reduce the impact of this sort of database misuse
slightly.
But really, people who are doing that sort of thing have no excuse for
themselves. They should be relegated to the same circle of hell as
people who think it's a good plan to write a crappy schema the first
time, because you can always optimise later.
A
--
Andrew Sullivan
ajs@commandprompt.com
+1 503 667 4564 x104
http://www.commandprompt.com/
Andrew Sullivan <ajs@commandprompt.com> writes:
> Oh, heaven. I can at least think of ways to use ENUM such that you
> can justify the trade-off. I can think of no excuse whatever for
> PQexec("COMMIT; BEGIN"). That's just lazy and sloppy.
> Note also that more recent releases, concurrent with the improvements
> to the drivers, also reduce the impact of this sort of database misuse
> slightly.
Actually, as of 8.3 I think the impact is zero, because of the lazy
XID allocation changes. It's still sloppy programming though.
regards, tom lane
* Thomas Mueller: > What do you think about it? Do you think it makes sense to implement > this security feature in PostgreSQL as well? Can't this be implemented in the client library, or a wrapper around it? A simple approximation would be to raise an error when you encounter a query string that isn't contained in some special configuration file.
This might seem sillly, but... ...are comments going to be considered statements for the purpose of this knob? (hoping the anwer is "yes") Darren
Darren Reed <darrenr@fastmail.net> writes:
> This might seem sillly, but...
> ...are comments going to be considered statements for the purpose of
> this knob?
> (hoping the anwer is "yes")
Are you trying to say we should forbid comments? No thank you.
regards, tom lane
fw@deneb.enyo.de (Florian Weimer) writes: > * Thomas Mueller: > >> What do you think about it? Do you think it makes sense to implement >> this security feature in PostgreSQL as well? > > Can't this be implemented in the client library, or a wrapper around it? > A simple approximation would be to raise an error when you encounter a > query string that isn't contained in some special configuration file. This could be implemented in a client library, but that means that you're still entirely as vulnerable; any client that chooses not to use that library won't be protected. It would be a mighty attractive thing to have something at the server level to protect against the problem. -- let name="cbbrowne" and tld="linuxfinances.info" in String.concat "@" [name;tld];; http://linuxdatabases.info/info/lsf.html If you add a couple of i's to Microsoft's stock ticker symbol, you get 'misfit'. This is, of course, not a coincidence.
Tom Lane wrote:
> Darren Reed <darrenr@fastmail.net> writes:
>
>> This might seem sillly, but...
>> ...are comments going to be considered statements for the purpose of
>> this knob?
>> (hoping the anwer is "yes")
>>
>
> Are you trying to say we should forbid comments? No thank you.
>
No.
When psql (for example) parses this:
COMMIT;BEGIN;-- Hi
it will generate individual commands with postgres (the server) over
the connection. ie. psql sends "COMMIT;" waits, then sends 'BEGIN;",
waits, etc.
When you do this in perl:
$db->prepare("COMMIT;BEGIN;--");
one single command string (the entire string above) is sent to the server.
How often do people code comments into prepare statements in perl
or the equivalent in java, ruby, etc?
Do you put comments in your perl prepare statements?
If comments count as a statement, at the server end, then the
multi-statement
disabling also disables another attack vector - slightly: you can no
longer attack
using this as your username:"' OR 1=1;--"
This raises another point - preventing muilti-statement attacks work so
long as the "hacker string" isn't broken up into multiple statements by the
client side. Passing said string to /bin/sh, which then passes it to psql
would successfully enable the attack even with this knob turned on. But
few people are likely to be using a design that is that slow.
Darren
Chris Browne wrote: > fw@deneb.enyo.de (Florian Weimer) writes: > >> * Thomas Mueller: >> >> >>> What do you think about it? Do you think it makes sense to implement >>> this security feature in PostgreSQL as well? >>> >> Can't this be implemented in the client library, or a wrapper around it? >> A simple approximation would be to raise an error when you encounter a >> query string that isn't contained in some special configuration file. >> > > This could be implemented in a client library, but that means that > you're still entirely as vulnerable; any client that chooses not to > use that library won't be protected. > > It would be a mighty attractive thing to have something at the server > level to protect against the problem. > If by "the problem" you mean SQL injection attacks in general, I strongly suspect you are chasing a mirage. Someone mentioned upthread that the best way to secure the database was to require that all access be via stored procedures. You can actually go quite a logn way down that road. I have seen it done quite successfully in a Fortune 50 company. In effect you are getting out of the game of catching insecure operations by not allowing your user direct access to them at all. (This approach also has huge benefits in allowing optimisation without disturbing client code). But this requires work by the DBA. I have some ideas about how we could make it a whole lot easier and more workable, but they probably don't belong in this thread. cheers andrew
-----BEGIN PGP SIGNED MESSAGE----- Hash: RIPEMD160 > How often do people code comments into prepare statements in perl > or the equivalent in java, ruby, etc? > > Do you put comments in your perl prepare statements? Does it matter? It shouldn't. They are comments. > If comments count as a statement, at the server end, then the > multi-statement disabling also disables another attack vector - > slightly: you can no longer attack using this as your username: > "' OR 1=1;--" Using placeholders and other best practices removes such attacks completely. I mostly agree with some other people in this thread that the 'disable multi-line switch' is marginally useful at best, and provides a false sense of security. But let's not confuse the issue with examples like the above. Otherwise I'll point out yet again that this whole things a solution in search of a problem. Poorly written apps will remain poorly written apps, no matter what server-side bandaids we try to apply. - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200805051559 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -----BEGIN PGP SIGNATURE----- iEYEAREDAAYFAkgfZzcACgkQvJuQZxSWSsjAoACg6UKhb2r94khikeOfT2cUOGhD vh0AoIY/8dSH4tkmsLxl2Jkpbn7/u3+4 =hGCo -----END PGP SIGNATURE-----