Security is a part of our work as developers. We need to ensure our applications against malicious attacks. SQL Injection is one of the most common possible attacks. Basically SQL Injection is one kind of attack that happens when someone injects SQL statements in our application. You can find a lot of info about SQL Injection attack. Basically you need to follow the security golden rule
Filter input
Escape output
If you work with PHP problably you work with PDO Database abstraction layer.
Let’s prepare our database for the examples (I work with PostgreSQL):
CREATE TABLE users ( uid integer NOT NULL, name character varying(50), surname character varying(50), CONSTRAINT pk_users PRIMARY KEY (uid) ) WITH ( OIDS=FALSE ); ALTER TABLE users OWNER TO gonzalo; INSERT INTO users(uid, name, surname) VALUES (0, 'Gonzalo', 'Ayuso'); INSERT INTO users(uid, name, surname) VALUES (1, 'Hans', 'Solo'); INSERT INTO users(uid, name, surname) VALUES (2, 'Luke', 'Skywalker');
OK our database is ready. Now let create a simple query
$dbh = new PDO('pgsql:dbname=db;host=localhost', 'gonzalo', 'password');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$stmt = $dbh->prepare('select uid, name, surname from users where uid=:ID');
$stmt->execute(array('ID' => 0));
$data = $stmt->fetchAll();
It works. We are using bind parameters, so we need to prepare one statement, execute and fetch the recordset. The use of prepared statements is strongly recommended. We can also use query() function:
$dbh = new PDO('pgsql:dbname=db;host=localhost', 'gonzalo', 'password');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$uid = 0;
$stmt = $dbh->query("select uid, name, surname from users where uid={$uid}");
$data = $stmt->fetchAll();
But what happens if $id came from the request and it’s not propertly escaped
$dbh = new PDO('pgsql:dbname=db;host=localhost', 'gonzalo', 'password');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$uid = "0; drop table users;";
$stmt = $dbh->query("select uid, name, surname from users where uid={$uid}");
$data = $stmt->fetchAll();
basically nothing: SQLSTATE[42601]: Syntax error. That’s because is not allowed to use two prepared statements in a single statement.
If we use an insert:
$dbh = new PDO('pgsql:dbname=db;host=localhost', 'gonzalo', 'password');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$uid = 20;
$name = 'Gonzalo';
$surname = "Ayuso'); drop table users; select 1 where ('1' = '1";
$count = $dbh->exec("INSERT INTO users(uid, name, surname) VALUES ({$uid}, '{$name}', '{$surname}')");
Now we have a problem. Our user table will be deleted. Why? That’s because of the user we are using to connect to the database. It’s important especially at production servers.
It’s very important not to use a database superuser in production. Superusers are very comfortable in our development servers, because you don’t need to grant privileges to every tables but if you forget this issue in production you could have Sql-Injection problems. The solution is very simple:
GRANT SELECT, UPDATE, INSERT, DELETE ON TABLE users TO gonzalo2;
And now
$dbh = new PDO('pgsql:dbname=db;host=localhost', 'gonzalo2', 'password');
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$uid = 20;
$name = 'Gonzalo';
$surname = "Ayuso'); drop table users; select 1 where ('1' = '1";
$count = $dbh->exec("INSERT INTO users(uid, name, surname) VALUES ({$uid}, '{$name}', '{$surname}')");
Now we are safe, at least with this possible attack.
Sumarizing:
- Filter input
- Escape output
- Take care about the database users. Don’t use one user that it allowed to perform “not-allowed” operations within our application. It sounds like a pun but is important.
Gonzalo Ayuso - Web Architect 
What about the use DELETE * FROM users; instead of DROP?
With a Delete your user table will be delted. OK maybe if we really want a read-only we will need GRANT SELECT ON TABLE users instead of GRANT SELECT, UPDATE, INSERT, DELETE ON TABLE users :). The idea of post is to take into account the database user. Especially superusers. dba users only for sysadmins. It’s not very common to need a db-user allowed to create/drop schemas (and sometimes I see it in some projects)
First filter input and then select a proper database user. Our level of paranoia is important here :). We always need to balance.
If you’re worried about the DELETE / UPDATE SQL-injection scenarios then you could create history tables for your sensitive data tables and populate them with triggers upon modification, like this DELETE example (mysql):
CREATE TABLE mytable_history (
id int(10) unsigned PRIMARY KEY NOT NULL auto_increment,
modified_date datetime NOT NULL,
orig_id int(10) unsigned NOT NULL,
col1 type,
col2 type …etc.
);
CREATE TRIGGER trg_mytable_history BEFORE DELETE ON mytable
FOR EACH ROW BEGIN
INSERT INTO mytable_history (modified_date, orig_id, col1, col2 …etc.)
VALUES (NOW(), OLD.id, OLD.col1, OLD.col2 …etc.);
OK. All depends on out level of paranoia (mine is usually high 🙂 ). The problem that I see with this kind of logs normally are great Disk-Space-Eaters. I haven`t ever use one trigger like that to monitor the history of modifications, but with similar things I’ve have problems. Those kind of problems easy to explain to managers: users cannot logging into the system because /log folder is full, or things like that 🙂
Also if the dbuser is allowed to drop triggers he can disable the trigger (paranoia leve = hight)
Anyway it’s a good idea to take into account
Sorry, but this is really bad advice.
Learn how to use prepared statements properly and you _never_ need to worry about Sql injection. I’ve never used PHP before, but I’m still sure that the above examples are not the correct way of using prepared statements.
Always use the “:ID” named parameters method in the first example above never use the “{$uid}” method later, and you should be fine.
You don’t need to filter/escape input, and you don’t need to configure special database users.
As I said the use of prepared statements is highly recommendable. I agree with you with this fact, but sometime we need to handle with legacy applications and we cannot rewrite all.
With the db-user it’s always important not to use a dba or super-user. For example I’ve seen one web application (couple of years ago) working with Oracle and using sys dbuser to connect to the database. The application works without problems, but obviously sys is not the correct user to run the application. If the application owner really want to use sys, go ahead, but he must know the security risks.
With this post I want to show how easy is to perform sqlinjection attacks. We don’t need to be super-hackers and we also don’t need to be super security experts to prevent them. Database super-users or dba are especial user only for sysadmins and database administrators. The applications always must run with non-privileges users. I like to work with three different kind of users. This is my personal layout:
dba: for database administration (to create databases, handling schemas, tablespaces, …)
developer: (create tables, views, …)
application: the end user (selects, inserts, update and deletes)
Also filter input should be always mandatory. No matter if we use prepared statement or not. Is one of the golden rules of web security and probably the most important one.
Filtering/database users is completely irrelevant for Sql injection – you should use prepared statements, and the problem goes away. I completely disagree about legacy code – either you can change the code, in which case fix it properly (ie using prepared statements), or you can’t fix the code and you’re screwed.
Sql injection is a solved problem, and we try to make sure that all developers know the correct solution – not suggest incorrect methods which only work in certain cases.
I’m agree with you with bind parameters. They are highly recommendable. Not only because of sqlInjection. They ever have better performance (https://gonzalo123.wordpress.com/2010/10/05/performance-analysis-using-bind-parameters-with-pdo-and-php/).
But we need to live with legacy code. It isn’t viable to rewrite and fix all software. I show you an example: If you want a blog, wordpress is a good solution. You can download install it at your server and use it out-of-the box. WordPress doesn’t use bind paramenters. OK it’s open source. You can fix it, but you need a lot time.
With this post I want to show the problem and give possible solutions. At least for me the privileges of the database users are very important. They’re also one of the typical things security auditors check when they scan system.
Anyway if the input variables are property filtered a lot of web security problems will disappear, but they still exist. We need to live in this scenario.
Yes parameter binding is the way to go. Unfortunately, thats not what you have done!!!
In this example:
$uid = 20;
$name = ‘Gonzalo’;
$surname = “Ayuso’); drop table users; select 1 where (‘1’ = ‘1”;
$count = $dbh->exec(“INSERT INTO users(uid, name, surname) VALUES ({$uid}, ‘{$name}’, ‘{$surname}’)”);
You have just done the same as you would have without any parameter binding. also, the second query would not work anyway with PDO, as it only ever allows SINGLE statements. so your observed behaviour of your table not being dropped is a coincidence.
This example should be performed like this:
$uid = 20;
$name = ‘Gonzalo’;
$surname = “Ayuso’); drop table users; select 1 where (‘1’ = ‘1”;
$query = $dbh->prepare(“INSERT INTO users(uid, name, surname) VALUES (:id, :name, :surname )”);
$query->bindParam(‘id’, $uid, PDO::PARAM_INT);
$query->bindParam(‘name’, $name, PDO::PARAM_STR);
$query->bindParam(‘surname’, $surname, PDO::PARAM_STR);
$query->execute();
PLEASE PLEASE PLEASE update your examples before someone database is comprimised!
The aim of this post post is not about bind parameters. Bind parameter are good. We all should use bind parameters always. The aim of this post is about the privileges of our database users. I prepare an example that drops the table (because of that I don’t use bind paramenters). If we use this bind parameters our table will be “safe” in this example. But as you said is safe only because of a coincidence. PDO only allow single statemnts. But with exec we can execute multiple statements: allow multiple statements + dba user + no filter input + no bind paramentes = problems
Exactly the same than if we filter properly our input parameters. There’s a lot of PHP applications that don’t use bind parameters (wordpress for example). If we connect the application with a dba user our whole database is compromised.
It seems that some people understand this post in the way: “select a good database user and you can use whatever your want with your code”. It’s definitely not the message. It’s more about “If you need to handle with legacy code ensure first your database user is a good one. Your database could be compromised”. Maybe I need to re-edit the post 🙂
Thats the point i’m trying to make…… your table is STILL not safe if you are not binding those parameters.
Imagine if the surname field had the value “smith’), (0, ‘h4x0r’,’0wn3d’); –“, now, thats not relying on a second statement, and if you’re relying on the user id to be 0 as an admin identifier (or you had a flag being set in the insert statement to state a users role) then you have just opened up the database to being exploited!
This coupled with XSS, makes the ideas posed later in this post quite dangerous, and waters down the good points you made, i.e. not using a db user with more privileges than are required.
Also, I wouldn’t hide behind “wordpress does it”, just look up wordpress on millw0rm, and see how frequently security issues arise with wordpress.
Gonzalo, I got your point.
However, let me point out that the heading is a bit confusing if not lier.
“How to protect from SQL Injection with PHP”
should be
“How to protect your DB with the right user privileges” since in your examples PHP is only the driving language, but the problem is founded in the use of PDO and the “solution” is in the DATABASE side.
Also, as many pointed out, the use of PDO you showed isn’t right (sorry for being evil).
As dealing with legacy code, it depends on the particular situation of every one, but in my case should be: “Look , this is crap. We MUST fix this in this way for this and this and it could imply the rewrite of this percentage of the system”
If no agreement, no job. I’ve already faced this kind of customers and the only earning was headaches and time immense lost and waste for me, and money wasting for the customer (as the product was left without proper fixing).
There will probably one willing to take the offer, but not me.
Regards