Page 1 of 2
PHP Database Class: Defending the data
Posted: Fri Oct 14, 2011 2:53 pm
by evstevemd
Hi,
I'm new to this forum and I have a question.
I have written my class on top of PDO and uses PDOStatement (prepare/execute). Now what I'm asking is, what else to do, as far as database is concerned, to make my database very secure (assuming all other attacks in an app like XSS have been mitigated). Should I add escaping of strings before preparing it?
Thanks

Re: PHP Database Class: Defending the data
Posted: Fri Oct 14, 2011 6:32 pm
by Mordred
Not having to escape data when using prepared statements is one of their major selling points. What you can do from now on for your DB security is never to use variables in the strings you prepare, you should only pass data in bind/execute.
Re: PHP Database Class: Defending the data
Posted: Sat Oct 15, 2011 8:17 am
by evstevemd
Mordred wrote:Not having to escape data when using prepared statements is one of their major selling points.
Hi Mordred,
should I escape variables with PDO::quote? PHP Manual recommends I use the PDO::PreparedStatement and says it is better. Have I missed your point?
PHP Manual wrote:PDO::quote() places quotes around the input string (if required) and escapes special characters within the input string, using a quoting style appropriate to the underlying driver.
If you are using this function to build SQL statements, you are strongly recommended to use PDO::prepare() to prepare SQL statements with bound parameters instead of using PDO::quote() to interpolate user input into an SQL statement. Prepared statements with bound parameters are not only more portable, more convenient, immune to SQL injection, but are often much faster to execute than interpolated queries, as both the server and client side can cache a compiled form of the query.
Not all PDO drivers implement this method (notably PDO_ODBC). Consider using prepared statements instead.
Mordred wrote:What you can do from now on for your DB security is never to use variables in the strings you prepare, you should only pass data in bind/execute.
Thanks for comment. Here is sample function
Code: Select all
public function fetch_one($table, $id){
//some code here
$stmt= $this->conn->prepare("SELECT * FROM $table WHERE $id_col[0]=:id");
$stmt->execute(array(":id"=>$id_val[0]));
$this->resultset = $stmt->fetch(PDO::FETCH_ASSOC);
}
I will move the $table into execute! Thanks
Re: PHP Database Class: Defending the data
Posted: Sat Oct 15, 2011 9:28 am
by evstevemd
evstevemd wrote:
Code: Select all
public function fetch_one($table, $id){
//some code here
$stmt= $this->conn->prepare("SELECT * FROM $table WHERE $id_col[0]=:id");
$stmt->execute(array(":id"=>$id_val[0]));
$this->resultset = $stmt->fetch(PDO::FETCH_ASSOC);
}
I will move the $table into execute! Thanks
I found that table name cannot be bound so I made a function to validate table name that it does only contain Aa-Zz 0-9 and characters -_
Code: Select all
private function validate_table($table) {
//validates that the table contains no character than A-Z 0-9 _-
$regex = '/^[a-zA-Z0-9_-]+$/';
if (preg_match($regex, $table)) {
return true;
} else {
return false;
}
}
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 7:08 am
by Mordred
evstevemd wrote:Mordred wrote:Not having to escape data when using prepared statements is one of their major selling points.
Hi Mordred,should I escape variables with PDO::quote? PHP Manual recommends I use the PDO::PreparedStatement and says it is better. Have I missed your point?
Nonono, you didn't parse my English (sorry): the point of prepared statements is that you don't have to escape data.
I found that table name cannot be bound ...
Aaaand you've found precisely what the problem with prepared statements is: they are a performance optimization and
not a security measure. If you want dynamic names in a query you're back to square one in the minefield with the biggest boots the army could find for you:
you have to step bloody carefully now .
Your situation is a bit different, since I don't think you should do any of the things you're doing -- not 100% sure since the code you pasted couldn't possibly work, but guessing your intentions here. I hope neither the table name nor the column name can possibly come from user input, since then you'll have a nice case of sql injection. Your validation function is not 100% true (check
http://dev.mysql.com/doc/refman/5.1/en/identifiers.html) but it's not a security issue.
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 9:30 am
by evstevemd
[0-9,a-z,A-Z$_]
Mordred wrote:
Nonono, you didn't parse my English (sorry): the point of prepared statements is that you don't have to escape data.
Sorry, parser error
Mordred wrote:Aaaand you've found precisely what the problem with prepared statements is: they are a performance optimization and not a security measure. If you want dynamic names in a query you're back to square one in the minefield with the biggest boots the army could find for you: you have to step bloody carefully now .
although it is dynamic query I use bind (with exception of a table). The Tables is checked before appended to query so there is no way for SQL injection through the variable.
Mordred wrote:Your situation is a bit different, since I don't think you should do any of the things you're doing -- not 100% sure since the code you pasted couldn't possibly work, but guessing your intentions here. I hope neither the table name nor the column name can possibly come from user input, since then you'll have a nice case of sql injection. Your validation function is not 100% true (check
http://dev.mysql.com/doc/refman/5.1/en/identifiers.html) but it's not a security issue.
I have a self made PHP MVC framework I use and it uses this DB layer with models to fetch data from DB. So the Model is the one specifying table name and other necessary data. So it is not user who supply directly but perhaps indirectly. So a table is specified in each Model (blogg model have blog table, forum model have forum table et al)
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 9:38 am
by Mordred
The most common situation in which you can have a vulnerable PDO code is when you want to have a sortable query, with the sort parameters given by the user.
Since you can't bind the column name to sort on, you need to include it dynamically in the query, and you need to take care to check if this is indeed a valid column, etc.
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 10:08 am
by evstevemd
Mordred wrote:The most common situation in which you can have a vulnerable PDO code is when you want to have a sortable query, with the sort parameters given by the user.
Since you can't bind the column name to sort on, you need to include it dynamically in the query, and you need to take care to check if this is indeed a valid column, etc.
Ok, now back to my case: Since table variable is not a threat (comes from model not user and is well validated and restricted to a given characters) and all other variables are bound to preparestatement, is there anything more I can do to protect data?
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 10:10 am
by Mordred
$id_col[0]?
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 10:35 am
by evstevemd
Mordred wrote:$id_col[0]?
you caught me, red handed
Ok, since this is column name, should limiting characters to [a-zA-Z0-9_$] be enough to prevent injection (since no quote is allowed) or should I take another measure?
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 11:12 am
by Mordred
The correct way to handle user input as column names is to check agains a whitelist.
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 11:25 am
by evstevemd
Mordred wrote:The correct way to handle user input as column names is to check agains a whitelist.
You mean regex like I do on table validation above?
Re: PHP Database Class: Defending the data
Posted: Mon Oct 17, 2011 4:33 pm
by Mordred
whitelist = a list (array) of possible permitted values.
Everything not in the whitelist is considered evil.
Re: PHP Database Class: Defending the data
Posted: Sat Oct 22, 2011 8:16 am
by evstevemd
Mordred wrote:The correct way to handle user input as column names is to check agains a whitelist.
Hi Mordred,
Do you mean I fetch all columns in a table and make a white list? Wouldn't that reduce perfomance?
Re: PHP Database Class: Defending the data
Posted: Sat Oct 22, 2011 12:59 pm
by Mordred
You can just hardcode them in your PHP source.