Page 1 of 1

A DAL-level SQL injection protection is a good idea?

Posted: Fri Sep 12, 2008 5:06 pm
by kaisellgren
Hi,

I'm done with developers of my project to write non-SQL-injection safe code. I want my DAL to do it.

Here's what I have done so far:

Code: Select all

$db -> prepare('INSERT INTO a (a,b,c,d) VALUES (?,?,?,?)');
$db -> bind($a,$b,$c,$d); // Another way is to separately put bind($a) bind($b) etc...
$db -> exec();
This way no one can insert non-SQL-injection safe code. BUT I WAS WRONG. Anyone can do:

Code: Select all

$db -> prepare('SELECT a FROM b WHERE c=$d");
$db -> exec();
Anyone might do the above which is obviously a huge danger. Instead he should do "WHERE c=?" and use bind() <_<

What can I do?

Re: A DAL-level SQL injection protection is a good idea?

Posted: Sat Sep 13, 2008 7:17 am
by Mordred
kaisellgren wrote:I'm done with developers of my project to write non-SQL-injection safe code. I want my DAL to do it.
Actually it's the only correct way of doing things securely, so well done.
kaisellgren wrote:What can I do?
On the PHP side - nothing. Anywhere that strings may go, someone may concatenate stuff into them. Prepared statements are not the panacea many people seem to think they are. Stored routines may serve you better at the price of some reduced flexibility.

The correct solution is on the meatware side - educate the coders, have code reviews and/or write automated tools to check the code.

Re: A DAL-level SQL injection protection is a good idea?

Posted: Sat Sep 13, 2008 8:18 am
by kaisellgren
Yes, that's a bit sad. There's always a easy way to mess up with vulnerable code.

Creating a system that checks if there are possible SQL injection vulnerabilities could be more secure, but more CPU intensive though.

I mean:

Code: Select all

 
INSERT INTO a (a,b,c,d) VALUES (sdfds,'sdfsf,45'45,dfg)
Then I would check that there are no invalid characters. For example I could check on DAL level if the metacharacters are escaped. But this becomes a bit slow and crazy.

EDIT: I got an idea. And I would like to hear your and everybody's opinion.

Let's say I want to insert into the db. The correct syntax is somewhat like this:

Code: Select all

INSERT INTO table (rowname,rowname2,...) VALUES (rowvalue,rowvalue2,...);
Whenever your prepare() the statement, the DAL would check whether the user has anything else except ? characters inside the VALUES -parenthesis. Basically this is invalid:

Code: Select all

$db -> prepare("INSERT INTO table (name,email) VALUES ('mordred','sorryforusingurname');");
$db -> exec();
The following code could throw an error, because you have inserted other than ? or , characters inside the parenthesis. I could basically try to make a little and as fast as possible check for common SQL clauses. This would prevent usage of the above code. Instead I would trigger an error "You are not querying the right way.. blah blah".

There's one problem, this is just a common case. Anyone could do something like:

Code: Select all

// Database manager plugin for my project
$table_to_manage = $_GET['table'];
$db -> prepare("SELECT * FROM $table_to_manage;");
$db -> exec();
The previous idea was good for only certain situations, but now I am dead. There's nothing I can do about the above code :'( ... ?

And btw, do you think using ¤ instead of ? as a 'metacharacter' is a good idea? Because ? might cause problems:

Code: Select all

$db -> prepare("SELECT * FROM table WHERE match REGEXP ('[a-z]?')");
$db -> exec();
I dont remember from the top of my head how the REGEXP clause in SQL went but if something like that is passed the ? could cause troubles, so using ¤ could be a good idea?