CakePHP sanitize parameter

Question

I have the following method in my CakePHP model:

public function login($login,$password){

    $arr = $this->find('first',array(
        'conditions' => array(
            'deleted' => 0,
            'online' => 1,
            'login' => $login,
            'AES_DECRYPT(UNHEX(password),'secretkey')=\''.$password.'\''
        )
    ));


    return $arr;
}

This method accepts two parameters ($login, $password) to authenticate the user.

I am wondering if this method is safe against SQL-Injection and other attacks.

If not, which is the best way to sanitize the input parameters using CakePHP?

I see that the Sanitize Class is deprecated as of 2.4.

Answer 1

Model::find() is only safe when used properly!

You must know that only values in key => value pairs are being escaped, keys and non/numerically keyd values are inserted into the SQL query as is!

Quote from the docs

CakePHP only escapes the array values. You should never put user data into the keys. Doing so will make you vulnerable to SQL injections.

http://book.cakephp.org/2.0/en/models/retrieving-your-data.html#complex-find-conditions

So your find() call as is, is unsafe and prone to SQL injections, it should instead look like this:

$arr = $this->find('first',array(
    'conditions' => array(
        'deleted' => 0,
        'online' => 1,
        'login' => $login,
        'AES_DECRYPT(UNHEX(password),\'secretkey\')' => $password
    )
));

That way the user input $login and $password is being escaped properly.

Answer 2

$Model->find should be safe, because it will escape the data before making the sql query.

http://book.cakephp.org/2.0/en/core-utility-libraries/sanitize.html#sql-escaping

Answer 3

The best way would be to use the built-in CakePHP auth that will do the work for you (see doc : http://book.cakephp.org/2.0/en/tutorials-and-examples/blog-auth-example/auth.html)

But if you need to do your own login function, Cakephp escape all parameter if you use built in methods like save() or find() ,... So, YES, your find() method is safe. (proven by the doc here : http://book.cakephp.org/2.0/en/core-utility-libraries/sanitize.html#sql-escaping)

For custom sql constructed by hand, you will have to escape manually your parameters because they aren't escaped by Cakephp