Best way to prevent sql injection?

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.

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 :slight_smile: ). 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

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 :wink:

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.

:slight_smile:

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?

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

First, if you’re executing one query, make sure that the user isnt executing multiple - search the string for a ; on RDMS that support multiple queries.

Database portability by switching classes is a myth. First, it would only work if you used very basic queries. Complex joins, table creations, anything remotely complicated will need to be tweaked for optimization at the least, others will just not work without a rewrite. When you switch DB’s, you’ll at the very least need to tweak all your queries to get the performance you had in the first, although you’ll find that some will need to be rewrote. If stored procedures and views are available, use them! If you dont, you’re hindering yourself.

With this fact in mind (that you’ll need to edit your queries at the least), use the available features of the DB to improve your application.

If you intend on switching to oracle, everything will need a massive rewrite.

I use DAOs and keep all my queries in one place - then, you load the correct set of DAOs depending on the database. This way, your business logic is independant of where the data came from.

God. I really can’t believe something can go on for ages just because of a question like this. Not to mention: “… that quoting is the absolute most simple way of making your queries bulletproof against any sql injection attack.” Quoting = bulletproof? Far from it. Putting quotes around a value won’t stop that value to have quotes inside.

I think the only thing that makes SQL injection so popular is because people don’t know it exists, even though they know, they don’t know how to properly handle it (oh yeah… by putting quotes around?)

On MySQL, use mysql_escape_string() on ANY data you want to input to the database. Even the ones not from the user. There are times you want to input exotic (i.e. containing quotes, binary data, etc.) so you should always quote things anyway. If you have magic quotes turned on, stripslash it first then mysql_escape_string it. magic_quotes_gpc DOES NOT ESCAPE YOUR DATA. Really. addslashes and mysql_escape_string works differently, they have different rules for different types of characters. And different databases work differently.

And yeah, you don’t stripslash on output. Why should you? The only reason why you should stripslash something is because you made it addslashes in the first place (because of magic quotes). If you retrieve the data from the database you don’t need stripslash, but in most cases you need htmlspecialchars (assuming you output as HTML).

If you use another database other than MySQL, please note that not all database drivers support an escaping function, at least oci8 doesn’t have one AFAIK. If you look at oci8’s manual or even some other database driver’s manual they blatantly use variables directly inside queries. Not sure if they intentionally teach you how to allow SQL injection in your scripts but I guess it’s their problem and your misfortune for following their paths.

In any case the better way is to use a good abstraction layer like MDB ( http://pear.php.net/package/MDB2 ). It has escaping function for all drivers even oci8, and you don’t even need to know “do I have to escape an int or not” because all you do is call $mdb->quote($variable, ‘integer’) and the driver will do it for you, quoting if needed or just plain casting to int if the database driver doesn’t like quoted ints. As simple as that.

After throwing the bulk of response above, now I’m questioning myself the whole meaning of life…

As a proof that addslashes() and mysql_escape_string() works differently, try inputing a binary data on your database. Yeah, binary, like an image or something (preferably not so small), use a blob field. If you only use addslashes() or rely on magic_quotes_gpc to escape your binary data most likely your query will choke. mysql_escape_string will work fine no matter what kind of data you gave to it.

The ultimate and only solution to SQL injection is [properly] escape your data using the proper function. The end. There’s nothing to it. You don’t have to check for ANY character whatsoever. Just pass it to the escaping function and you’re done. There’s no SQL injection will get away with it… not unless the escaping function is buggy.

The same thing goes for XSS (cross-side scripting), if you always use htmlspecialchars() for ANY kind of output, there is NO WAY someone would cross-side script in your page, as everything is printed verbatim. Of course when you need to process HTML tags things get a lot more complicated…

ceefour: noone ever stated that quoting would prevent injection. But quoting ESCAPED variables of ALL types in EVERY query is probably 100% injection-proof.

I agree with the following fact: tweaking queries is almost inevitable (be it for LIMIT or the use of RegExp with LIKE) if you have to move to a new db.

I still don’t see why it makes the use of quoted int something good. It might sound like a poor argument, but it simply can’t get any richer. Yes LIMIT is plain MySql-ish, yes we use LIMIT a lot. Is that to say that we can use quoted integers? Mmhh… I missed a logical link. But at the end of the day, each one its own.

Good practice is obvious in this case: you will have to tweak more queries. On a project with many queries, that can become quite a pain.

:slight_smile:

To explain why I think quoting is just as good as variable verification;

Let’s say we have a query
SELECT * FROM article WHERE article_id = %article_id%
where article_id is obviously an int field.

Let’s suppose that article_id comes in this script as a part of a request. Now imagine a script-kiddie that tries to hack into it by posting article_id=‘5; DELETE * FROM article;’, (without the quotes).

Now we have these scenarios:

  1. No type verification, no casting, no escaping, no quoting
    Query: SELECT * FROM article WHERE article_id = 5; DELETE * FROM article;
    Risk: all fields

  2. Just escaping
    Query: SELECT * FROM article WHERE article_id = 5; DELETE * FROM article;
    Same as (1), sincer no character is escaped.
    Risk: all non-quoted fields (ints, floats, …)

  3. Escaping and type verification
    Verify the incoming parameters with is_int() and such.
    Script refuses to run the query, since the parameter is not an integer.
    Risk: none
    Pros: more cross-db compatible, DB isn’t bothered by invalid queries
    Cons: Have to type-check every incoming variable, that will not be quoted in the query

  4. Escaping and quoting
    Query: SELECT * FROM article WHERE article_id = ‘5; DELETE * FROM article;’
    Query fails on execution.
    Risk: none
    Pros: Easy and unified use; every variable gets escaped and quoted, no exeptions.
    Cons: Presumably not compatible with DBs other than mysql/postgre (dunno really). Also the DB has to execute the query, even though the script could already stop execution earlier.

By escaping, I mean the proper escaping, depending on a database (NOT addslashes). I use ADODB, which (as probably every DB abstraction layer does) has a function to escape a variable, using the specific syntax of the db in use.

Neither (3) nor (4) have that much of advantage to dump the other on account of some prejudice. (4) is easier to use, (3) is more strict, but both disable any user-input injection. Also, example (2) makes it very clear: just escaping the incoming variables is not even remotely secure.

Regards.

Very interesting and instructive post. You won’t make me use quoted int but… I must admit that your ways definitly make sense.

:slight_smile:

What happens if the user posts something like:


5'; DELETE * FROM article;''

Wouldn’t 4) turn into:


SELECT * FROM article WHERE article_id = '5'; DELETE * FROM article;''

Or are ’ among the characters that are escaped?

Unless you’re using LIKE and similar feautures with its own metachars, for example:

DELETE FROM tbl WHERE name LIKE ‘<user-input>%’

Just escaping <user-input> doesn’t give enough protection

The most powerful and safe method you’ve forgotten is to force type instead of checking it:

sprintf(“SELECT * FROM article WHERE article_id = %d”, $iser_input);

solves the problem.

Depends on the database; it’s escaping and quoting function. In Mysql this query would be
SELECT * FROM article WHERE article_id = ‘5\’; DELETE * FROM article;\‘\’’
since the ’ is quoting character and is escaped.

I think in some databases (not sure, but possibly Interbase-based (includes Firebird)) the quoting character is also ‘, but escaping is done with another ’ added in front of it, resulting in
SELECT * FROM article WHERE article_id = ‘5’’; DELETE * FROM article;‘’‘’’
(all quotes are single)

And some (MSSQL maybe?) use " as quoting character, so ’ doesn’t have to be escaped, the query is:
SELECT * FROM article WHERE article_id = “5’; DELETE * FROM article;‘’”
(first and last quotes double, other single)

All goes well if you use proper (that is, database-specific) functions: for escaping and quoting.

I am not really aware of which quoting/escaping syntax is used by other databases other than MySQL and PostgreSQL, so the speculations about Interbase and MSSQL are just that - speculations.

stereofrog:
In your case (remember that request variables are always strings), the possible situations are as follows:

  • $user_input is a string containing an integer, i.e. ‘1324’ => properly converted
  • $user_input is a string beginning with an integer, i.e. ‘12x34’ => converted to 12
  • $user_input is a string not beginning with an int, i.e. ‘abc 243’ => converted to 0
    All resulting in a valid (erroless) query, but with possibly unwanted behaviour, since the query is actually executed, it does not fail. This would result at least in retrieval of wrong data (or no data at all), but no database error.

Regards.

I feel I’m missing the point here. Are we talking about SQL injection, input validation or runtime typechecks? These are different things and it would be unwise to confuse them.

Again, try this
SafeSQL class
“SafeSQL - an SQL query processer to automate the tedious tasks of syntax
testing, injection attack-proofing, dropping parts of queries and other
misc features. It has only been tested with MySQL syntax, but any ANSI
SQL-92 compliant db library should work OK.”

Also have a look at this sitepoint article for more examples of SQL Injection Attacks and you will see that merely quoting ints is not going to do it.

I’m a little confused over this whole subject. According to Harry Fuecks:

http://www.webmasterstop.com/63.html

Once the variable (with addslashes) has been entered into the db, the extra slashes (or escaped values) are discarded. So, I just set up a db to test this and sure enough, all my escaped strings were entered into the db without the “/”.

So, as this is the case, why would you ever need to use strip_slashes() as there are no slashes to be stripped from the data?

Also, when you are adding addslashes to a variable to add the data to the db, which way is the correct way:

  1. $sql = “insert into…blah… addslashes($myvar)”;

or

  1. $sql = “insert …”;
    $sql = addslashes($sql);

Thanks for the help and insight!

Rob.

Gotcha dbevfat.

@stereofrog:
SQL injection is a result of not validating input and/or not doing runtime type checks. Although in this case runtime type checks are part of the validation process. If we’re supposed to get only integers, we make sure (validate) that this is the case with runtime type checks.

add slashes only to the variable you want to clean (1 is the better solution, but it’s even better to use the function outside your query for legibility issues).

strip_slashes() has been created to counter the effect of having magic_quotes_gpc() turned on. Two slashes would be added if you used addslahes() as with magic_quotes_gpc() turned on. With strip_slashes, you can virtually disable magic_quotes_gpc(), thus working in any environment:


function strip_magic_quotes($arr)
{
	foreach ($arr as $k => $v)
	{
		if (is_array($v))
			{ $arr[$k] = strip_magic_quotes($v); }
		else
			{ $arr[$k] = stripslashes($v); }
	}

	return $arr;
}

if (get_magic_quotes_gpc())
{
	if (!empty($_GET))    { $_GET    = strip_magic_quotes($_GET);    }
	if (!empty($_POST))   { $_POST   = strip_magic_quotes($_POST);   }
	if (!empty($_COOKIE)) { $_COOKIE = strip_magic_quotes($_COOKIE); }
}