Best way to prevent sql injection?

[redbaron]

What are your methods for preventing sql injection? For example, say I am adding $value into my database. What is the best way to make sure that if $value contains " ' " it is inserted itno the database, but does not break the query?

[dbevfat]

by checking get_magic_quotes_gpc() you can find out whether the incoming variable was escaped (some characters got a \ in front of them) or not.

Now that you know that, you can clean up the variable (if it was escaped) by calling stripslashes(), so that you definetly have an unescaped string.

You must not insert raw (unescaped) variables in the database, so you have to escape it again, this time using the proper function for the escape to be valid (different databases use different escaping), for mysql that would be mysql_real_escape_string().

I am using ADODB library for DB access and use it's methods, which internally call a proper escaping function, so that my code is as database-independant as possible.

Regards

[cringer]

addslashes http://us2.php.net/manual/en/function.addslashes.php

I think some say this one is better:

mysql_escape_string http://us2.php.net/manual/en/functio...ape-string.php

be aware if magic quotes is turned on

http://us2.php.net/manual/en/functio...quotes-gpc.php

[samsm]

Addslashes is vital, but still isn't a brick wall against injection attacks.

For example:

SELECT * FROM TABLE WHERE privilages=1 AND article=[user-entered]

Could be altered to:

SELECT * FROM TABLE WHERE privilages=1 AND article=22 OR privilages=123

This version doesn't suffer from that exact problem:

SELECT * FROM TABLE WHERE (article=[user-entered]) AND privilages=1

Basically, it pays to think about how the user-entered portion of a query might be manipulated. A more simple solution than the above would be to check to make sure the user enterd value was numeric, or use some non-standard SQL (that happens to work with MySQL) and surround the number with quotes.

[cringer]

If you know in advance what the value is supposed to contain or not contain, then by all means make sure it is valid before proceeding to involve it with your DB. Always.

[geniusgoalie]

It really depends on the querys. There isn't a prevent_sql_injection function (although I really wish there was!)

[samsm]

It really depends on the querys.

Very true.

It is probably possible in some database setups to make sql injection attacks impossible, even without using addslashes or any sort of validation. For example, if you put limitting conditions in a join rather than in a where clause. I'm not saying that you should rewrite your queries just so that you don't have to check input, but I think it is interesting to muse about.

[Roger Ramjet]

If you read the manual on sql injection you will see just how firghteningly easy it is to do, and just how extensive the damage can be.

The first solution is to create a mysql user with read-only access to only limited columns and tables and to use this user account to connect for the queries for your dynamic page content and user search functions. This will hide much of your schema from the public as well as preventing many of the common attacks like add user or extend privileges.

Then create other specific user accounts with write access to only those table columns that absolutly demand it. Where possible, place all pages that save user input into protected folders that require authentication to access.

Wherever you have to accept user input data for addition to the database, do all the things like mysql_real_escape_string, but also test the input with regular expressions etc that look for SQL operators and clause constructions. Then save the input into work tables in a seperate database so that your main database is not subjected to the attack directly. Finally use private scripts that transfer the input from the intermediate database to your main database. If the work database has been successfully attacked these scripts should fail and then alert you.

[XtrEM3]

Since i don't need to sort anything or do any searches, i convert everything to hex using bin2hex(), then insert it into the db.

I'm sure this isn't the correct way to do it...but if it works, why not.

[samsm]

Since i don't need to sort anything or do any searches, i convert everything to hex using bin2hex(), then insert it into the db.

What are the benefits of doing that?

[geniusgoalie]

What are the benefits of doing that?

I was wondering the same thing... although he is not the first person I have heard that does this.

[dbevfat]

...
SELECT * FROM TABLE WHERE privilages=1 AND article=[user-entered]

Could be altered to:

SELECT * FROM TABLE WHERE privilages=1 AND article=22 OR privilages=123
...

I believe that as long as you verify the incoming data, the first query is also injection-proof. If the parameter is supposed to be an integer (as it is in this case), a simple is_int() or a cast would prevent the query to be executed. As for the string values, they are already escaped when they enter the query (assuming some safe sort of escaping is used), so it's safe again.

And also, if your database supports quoted integer parameters in query, it's probably a good practice to use this method. In that case, the query

SELECT * FROM TABLE WHERE privilages=1 AND article='[user-entered]'

Could only be altered to:

SELECT * FROM TABLE WHERE privilages=1 AND article='22 OR privilages=123'

which would fail again. Dunno about other databases, but Mysql and Postgre both support such syntax.

[XtrEM3]

What are the benefits of doing that?

well, hex is all numbers no matter what, so they can input any character they want, and it wont break a URL, mysql query, or anything. You dont have to worry about escaping characters, tags, etc.

Works really well for me.

[sid egg]

All you really need to do is watch for things like overflows (just substr everything, just to be sure... never rely on anything except your own code) and addslashes EVERYTHING that touches a database.

[polvero]

i'm not sure how much we can go on about a topic like this.
it's more like, you learn how to do it right...then you do that for all your websites.
mysql_real_escape_string has always done the trick for me.

it's generally good to turn off magic quotes. this way you have a little more power over what happens by default.

just remember the rule.

addslashes() on input.
stripslashes() on output.

[Kilroy]

mysql_real_escape_string()

[duuudie]

Here are the functions I use all the time. You will notice that I don't check for datatype in these functions for various reasons (due to conditionnals that are specific to some portions of the code --> checks made in the relevant scope).

//check if incomingData is not empty and of the expected length

function checkIncomingData($idata, $minsize, $maxsize) 
{

   if ( 

   
         strlen($idata)<$minsize

   or  
   
         strlen($idata)>$maxsize 
      
      ) 
   {
      
      return false;
   
   }
   
   else
   
   {
      
      return true;
   
   }
}

//make sure that nothing bad can be entered by the user (-->sql injection attack)

function cleanIncomingData($idata) 
{

   $cleaned = trim($idata); 
   $cleaned = mysql_real_escape_string($cleaned);

   return $cleaned;
} 

Just a little note: mysql_real_escape_string() is a little more thorough than addslashes().

[Roger Ramjet]

I believe that as long as you verify the incoming data, the first query is also injection-proof.
... Postgre both support such syntax.

RE: Postgre, from
the manual

Example 27-2. Splitting the result set into pages ... and making superusers (PostgreSQL)
<?php

$offset = $argv[0]; // beware, no input validation!
$query = "SELECT id, name FROM products ORDER BY name LIMIT 20 OFFSET $offset;";
$result = pg_query($conn, $query);

?>
Normal users click on the 'next', 'prev' links where the $offset is encoded into the URL. The script expects that the incoming $offset is a decimal number. However, what if someone tries to break in by appending a urlencode()'d form of the following to the URL

0;
insert into pg_shadow(usename,usesysid,usesuper,usecatupd,passwd)
select 'crack', usesysid, 't','t','crack'
from pg_shadow where usename='postgres';
--

If it happened, then the script would present a superuser access to him. Note that 0; is to supply a valid offset to the original query and to terminate it.

This works because Postgre supports multiple queries.

_______________

Escaping quotes is so that the user cannot include one in the input to terminate the input string so that any additional SQL clauses or statements will be processed. But that is only one way to mess with your DB as the above example shows.

geniusgoalie It really depends on the querys. There isn't a prevent_sql_injection function (although I really wish there was!)

Have a look at this SafeSQL class

[dbevfat]

...
If it happened, then the script would present a superuser access to him. Note that 0; is to supply a valid offset to the original query and to terminate it.
______________

Escaping quotes is so that the user cannot include one in the input to terminate the input string so that any additional SQL clauses or statements will be processed. But that is only one way to mess with your DB as the above example shows.

I think you misunderstood me. My point in the previous post was to enclose variables in quotes even if they are of type int, and escape the contents.
Your code would become

<?php
$query  = "SELECT id, name FROM products ORDER BY name LIMIT 20 OFFSET '$offset';";
?>

Note using the quotes around the $offset in the query. Now whatever $offset might user set, its either a valid offset (thus a valid query) or and invalid query, but you cannot inject any sql code in here. Provided the $offset is escaped, as I stated in my previous post.

Regards

[Roger Ramjet]

But that all depends on whether your db engine will accept quoted integers, some will some wont.

PS not 'my' example, from the manual.

[duuudie]

why would anyone use quote around integers?

Suppose you must move a db that doesn't let you go away with it (not like mySql does), what will you do? If you're using a class to deal with your db, it's a matter of seconds to switch to a new db. With quoted integers, you might end up re-writing all your queries.

[dbevfat]

But that all depends on whether your db engine will accept quoted integers, some will some wont.

Indeed. With the databases that do not accept that, integers should at least be type-casted or verified with is_int(). But is that enough? At a first glance it would seem it is, since any injection code results in a 0 (zero) when casted, or a failure to satisfy is_int().

duuudie:
Depends. Some applications I've written will certainly not be ported to other databases. Some have already been ported from mysql to postgre and both support such syntax. It'd be good to know for other databases as well.

I agree on writing queries as standard as possible, but there is no such thing as a standard SQL, well, there's ANSI SQL, but every database uses it's own SQL. So, at some point of complexity, queries will always have to be rewritten when porting. Except for the simpliest SELECT and probably most of INSERT/UPDATE statements.

With PostgreSQL, we use a method of reads-thru-views and writes-thru-stored-procedures (I guess there must be a fancy name for that ). The stored procedures can easily be programmed in such manner that they accept everything in quotes (varchars) and these procedures then cast and actually perform the inserts/updates in the database. For the web-client part, that's safe. All incoming data is escaped by php and stored procedures are called as were supposed to be - with quoted parameters only.

Regards

[samsm]

why would anyone use quote around integers?

Because...

$user_input = addslashes($user_input);
$sql = "SELECT * FROM tableX WHERE articleid='$user_input' AND privs=4"; 

... that quoting is the absolute most simple way of making your queries bulletproof against any sql injection attack.

It's a crutch, like magic_quotes. I don't use either, but they are nice tricks for people who aren't very experienced. By the time someone knows enough to know that those aren't the best way to work, they also probably have the experience neeeded to avoid SQL injection.

[duuudie]

Because...

$user_input = addslashes($user_input);
$sql = "SELECT * FROM tableX WHERE articleid='$user_input' AND privs=4"; 

... that quoting is the absolute most simple way of making your queries bulletproof against any sql injection attack.

It's a crutch, like magic_quotes. I don't use either, but they are nice tricks for people who aren't very experienced. By the time someone knows enough to know that those aren't the best way to work, they also probably have the experience neeeded to avoid SQL injection.

Well... I strongly disagree with you on that

I was lucky enough to have people to tell me not to do it, here on SPF, when I started learning PHP one year ago. Is the fact that you're not very experienced a good reason to do that? I don't think so. You either know that int shouldn't be quoted or you don't. Once you're aware that it's not a good practice, just get rid of it.

[dbevfat]

I am far from a beginner in PHP, but I still fail to see a very good argument, why int parameters shouldn't be passed as quoted strings. If the issue is db compatibility, it fails anyway, the moment you use SELECT LIMIT or anything more complex than a really basic query. Sure, you have to fix MORE queries if you quote ints, but here you go, fixing almost all queries anyway. Unless you use some query builder db-independant layer (anyone knows any good one?).

Besides, lots of DBs support functions (Mysql soon), and by using db-functions to inset/update data in the database, you can supply quoted-params to the function, then typecast inside, and perform the change.

I really don't see a problem here, compared to what can this do for security. Am I missing something really big, can anyone point it out?

I was lucky enough to have people to tell me not to do it, here on SPF, when I started learning PHP one year ago. ... You either know that int shouldn't be quoted or you don't. Once you're aware that it's not a good practice, just get rid of it.

This is a very poorly argumented statement. Can you be more specific of what 'good practice' means?