I have been using CakePHP for a long time now and enjoy every second. It provides a productive, easy to use and well document platform for PHP application. The key advantages for me are – transparent OR mapping, a strong Model View Controller framework, and tons of extra utilities that make your life better.
I have came across a possible security issue in one of cakePHP code examples. This section of code is responsible to authorize or un-authorize clients access to a certain action (MVC flow)
action == 'delete') { if ($this->Auth->user('role') == 'admin') { return true; } else { return false; } } return true; } ?>
The major security rule this code is breaking is – never ever have ‘return true’ as a default for an authorization method.
Security driven operations should always return “unauthorized” as a default value. You should always take the white-list-approach to authorization methods, in other words you want to allow privileges to specific scenarios and block all other scenarios by default.
This is how this method should be implemented:
action == 'delete') { if ($this->Auth->user('role') == 'admin') { return true; } } if ($this->action == 'view') { return true; } ... return false; } ?>
A good way to prove this is a safer implementation would be to think what will happen if we add a new action (let’s call it “edit”) and let’s say we forget to rewrite the isAuthorized method.
- Original implementation: The ‘edit’ method would be accessible to any role in the system and the developer would have hard time discovering that he/she forgot to rewrite the isAuthorized method. This security hole might never be found until it is too late.
- New implementation: The ‘edit’ method will not be accessible to any role and the developer would immediately know that there is something he forgot to do.
I have submitted the correction to the official CakePHP tutorial and hopefully they will amend the reference code shortly.