When building applications is always smart to check and clean the user input. This is a must when you building a website or public application.
I create always an instance of Sanitize class in my AppController and then using it in all of my controllers like this
uses('Sanitize');
class AppController extends Controller {
function beforeFilter(){
$this->cleaner = new Sanitize();
}
}
?>
in the controllers I am using:
But you should know that Sanitizer uses mysql_real_escape_string() function.
mysql_real_escape_string — Escapes special characters in a string for use in a SQL statement. This function must always (with few exceptions) be used to make data safe before sending a query to MySQL.
What is the problem?
It’s mentioned here – it replaces \x00, \n, \r, and \x1a as pure strings. So, all new lines line will be replaced with ‘\n’. This is problematic when you use Textarea fields (i.e. Comments form) and every time when you want to display the information, you need to replace ‘\n’ with “\n”.
My quick and dirty solution for this problem is:
class AppModel extends Model {
...
public function find($type, $options = array()) {
$result = parent::find($type, $options);
$result = $this->unclear($result);
}
...
function unclear($data){
if (empty($data)) {
return $data;
}
if (is_array($data)) {
foreach ($data as $key => $val) {
$data[$key] = $this->unclear($val);
}
return $data;
} else {
$data = str_replace('\n', "\n", $data);
$data = str_replace('\r', "\r", $data);
return $data;
}
}
}
As you can see in my AppModel, there is a recursive function unclear() and I am using it every time when a find method is called. As I said – it’s quick and dirty and possible there is a better way of doing it, but it’s working.
Few things:
If you use the model methods for saving data, you don’t need to run the clean method — your data will automatically be cleaned against SQL injection. The sanitize class is really meant for cleaning data that is being retrieved from your database. More here:
http://cakephp.prometsupport.com/2008/handling-sql-injection-in-cakephp/
As for your implementation, you can just call Sanitize’s methods statically:
Sanitize::clean( $some_string );
This will keep your app_controller code a bit cleaner and easier to read, and only load the class when used in a specific action.
Also, rather than extending your Model::find method, I’ve found it preferable to just create a helper with a method for outputting user generated text that replaces newline characters with breaks, and htmlspecialchars the text. Something about changing such an important method kinda disagrees with me!
Batter, I agree it’s hard to make SQL injection, with Cake if you use find() and save() functions, but clean also prevent for XSS attacks – for example Comments in a blog. So basically they need to be cleaned again this.
The problem in my case is that I’ve used this in many controllers in my project and later on I discovered this problem, so the choice was to go to all 60 controllers and ~(60×3) views and to change the code, or to create this function 🙂
Hey Nik:
I think we are touching on the age-old ‘to store dirty data or not’ debate. I would prefer to keep the XSS attempt in my database so I can keep record of who was trying to leave XSS messages in blog posts, just always being sure to run clean on the data before it’s written to the page… Either method obviously ends with the same result — more secure code which is all that really matters.
Batter, you are completely right!