0

I have been reading this:

How can I prevent SQL injection in PHP?

and I'm thinking of using this whitelist strategy to create very dynamic mysql statement for CRUD.

so my idea is to build 4 functions, buildSelectStatement, buildInsertStatement, buildDeleteStatement, buildUpdateStatement and each function will help me to build the sql statement. For example, the "buildSelectStatement" will take the following arguments:

$selects, $whitelist_selects, $where, $whitelist_where, $orders, $whitelist_orders, $order_syntax, $whitelist_order_syntax, e.g:

$whitelist_select = array("id", "username", "hashed_password", "creation_date", "any other columns in my table"); //all columns in table
$selects = array("id", "username"); //contains fields I want to select
$whitelist_orders = array("creation_date");
$orders = array("creation_date");
$whitelist_order = array("id", "username", "creation_date"); //fields that can be sorted
$order_syntax = "DESC";
$whitelist_order_syntax = array("ASC", "DESC");
$where = ...
... ...

then inside the function I'll use array_search to compare $whitelist_select against $selects, $whitelist_orders against $orders etc. to help me to build a dynamic statement like:

SELECT `id`, `some_field` FROM user_table WHERE `username` = :username
SELECT `hashed_password` FROM user_table ORDER BY creation_date DESC

then I'll create a generic function to take the statement and execute it. ie.

//I used buildSelectStatement() to get $query as well as $bind_array
protected function getSelectResult($query, $bind_array) {
    $this->stmt = $this->dbh->prepare($query);
    foreach ($bind_array as $param=>$value) {
         $this->stmt->bindValue($param, $value, findBindType($value));
    }
    ...
    //execute
    //then return result
}  

is this safe? is there any thing I should worry about?

Community
  • 1
  • 1
Josh
  • 700
  • 2
  • 8
  • 36
  • It is unusable. Why do you want to "build" a query? Why can't you just write it as is? – Your Common Sense Feb 28 '14 at 21:35
  • 1
    @YourCommonSense: why write sql when you can write a few million function calls and pass around obscure arguments so the code can write sql for you? It's obviously so much more enterprisey. – Marc B Feb 28 '14 at 21:36
  • then I'll have to write a lot of statement. ie. Select `id`, `hashed_password` FROM user_table WHERE `username` = :username (for login purpose) Select `id`, `username`, `email`, `address1` From user_table WHERE `username` = :username (for displaying user data based on username) SELECT `id`, `username`, `email`, `address1` FROM user_table WHERE `creation_date` > :creation_date (check which users are created after certain period of time) and so on. I thought I should try not to create redundant code and I should reuse my code as many as I can – Josh Feb 28 '14 at 21:42
  • As soon as you'll start considering it, you will find that you either didn't save at all or that you **obfuscated** your SQL, making it totally unreadable. – Your Common Sense Feb 28 '14 at 21:49
  • @YourCommonSense what I don't understand is I thought your original comment http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php/8255054#8255054 suggested that whitelist can be used to build dynamic sql? please advise. – Josh Feb 28 '14 at 21:57
  • 2
    dynamic SQL means you don't know beforehand which query it will be. Say, there are conditional where conditions based on user input or custom ordering based on user input - in these cases you CANNOT avoid a dynamically built query. While your queries like `id, hashed_password FROM user_table WHERE username = :username` are perfectly static and can be written as is - so, there is no point in building them dynamically: you will gain nothing but lose readability, flexibility and reliability. – Your Common Sense Feb 28 '14 at 22:07
  • http://stackoverflow.com/a/18327617/285587 is a similar question – Your Common Sense Feb 28 '14 at 22:09
  • okay, I'll just write my sql statement as is, but what about my function getSelectResult($query, $bind_array)? can I write this generic function to retrieve results from database? is there any security issue I should know? – Josh Feb 28 '14 at 22:12
  • I encourage people in writhing a function like this - and you can find an implementation by the above link. As long as you can add variables via placeholders, there are no security concerns at all. However, I'd suggest to create a set of functions, for the different result types – Your Common Sense Feb 28 '14 at 22:15
  • okay, thanks, will do – Josh Feb 28 '14 at 22:20

0 Answers0