Saving Model Data and Security

In google groups there are quite a few discussions every month about security against (primary key) injection, xss or other things.
And yes, the default templates will not protect you from any of this. They are meant to produce quick "standard" output. But they are not prepared for the real world, at all.
It is actually quite irresponsible not to use custom templates or manually adjust the forms/views.
UPDATE: I could finally convince them to use h() in their default templates! A big WIN for security!

A while ago i wrote about how to create custom templates. See this article for details.
You will also notice, that the controller actions and views are protected against basic – and only basic – injections.
That is mainly xss-attacks. By using h() in the views we can make sure that dangerous strings get destroyed. The custom templates also ensure, that the record we edit/delete exists.

What it still doesn’t do is protecting the model data.

Note: some of the following could be achieved by adding the security component. But sometimes, it is not possible or not desired.

Protection against too many fields

Another tricky one. Again, Firebug can be used to violate your forms. Lets say, a user wants to have more rights – and he happens to know that User.role_id must be 2 instead of 1 in order to be "admin".
In the User edit form (besides "email", "birthday" and stuff) he could then inject a form input called "role_id" which usually is not there.
With a normal $this->save() operation this would actually work! After the next login his role would be 2 (= admin).
What can we do about something like that?

Cake has a "fieldList" as third parameter of the save() function build in:

if ($this->User->save($this->data, true, array('id', 'gender', 'birthday'))) { # note the id (needed here)

Id, gender and birthday are passed, any other field in $this->data will be omitted (especially our role_id^^).
I use this in all sensitive forms (user and role related).

Careful: If you don’t pass the id, it will be omitted and the model cannot set its primary key (and might perform "add" instead of "edit"!

Primary key injection on edit actions

This is the not quite the same as the above topics. All edit actions have the primary key in the forms. I usually remove them, but sometimes I am too lazy to do that (because you need to alter the form url then as well).
So the id actually exists – but won’t be checked prior to the save.
Well, here are some custom solutions – there are more general ones, of course. But I just want to point out the problem and a way to fix it. These could be added to your bake templates, for instance.

Lets take our custom template from above (see link) and modify it:

public function edit($id = null) {
    if (empty($id) || !($data = $this->User->find('first', array('conditions' => array('User.id' => $id)))) {
        //error flash message
        $this->redirect(array('action' => 'index'));
    }

    if (!empty($this->data)) {
        # solution 1: just override with the "correct" one:
        $this->data['User']['id'] = $id;

        # solution 2: check against "correct" one
        if ($this->data['User']['id'] != $id) {
            //BLACKHOLE
        }

        if ($this->User->save($this->data)) {
            //OK
        } else {
            //ERROR
        }
    }
}

Solution 1 is self-explanatory and cannot go wrong.
Solution 2:
After we made sure the record exists, we check the passed id against the record id.
If they don’t match someone tries to trick us. Otherwise the record can be saved.
Solution 3:
Remove the id from the passed arguments (third parameter of save() – but makes it more complicated)
Solution 4:
Put the check in beforeValidate() of your app_model.php. Usually – with the above template you have just "read" the record, so the model has the id in $this->id. It could then return false if the passed id and $this->id do not match. Thats actually my favorite. But I am not sure if it works in all cases. You may only check if you are actually saving a record – meaning: both ids have to be not empty. Only then it makes sense to apply this specific piece of validation.

Protection against missing fields

This is a tricky one. Most beginners (I didn’t either) dont realize that a form can be modified with tools like and Firebug (Firefox Addon) and that Cake would not bother.
Lets say we have a registration form and your gender has to be provided. But you don’t want to. So you simply remove this input from the form and submit it. Violà – it worked!

Why does it work? Cake automatically applies the validation "needed" for this form. All fields that are passed (it doesn’t matter if they are empty or not – they just have to exist on submit) will be checked. Validation rules for fields that did not get passed will be omitted. And thats our problem. Right here.

OK, we need to make sure that the form fields are actually part of the model data before the validation kicks in.
I wrote a app model extension which can be used – you could also write a model function which one by one adds the required fields if the field does not exist.

Put this in your app model:

/**
 * set + guaranteeFields!
 * extends the core set function (only using data!!!)
 * 2010-03-11 ms
 */
public function set($data, $data2 = null, $requiredFields = array()) {
    if (!empty($requiredFields)) {
        $data = $this->guaranteeFields($requiredFields, $data);
    }
    return parent::set($data, $data2);
}

/**
 * make sure required fields exists - in order to properly validate them
 * @param array: field1, field2 - or field1, Model2.field1 etc
 * @param array: data (optional, otherwise the array with the required fields will be returned)
 * @return array
 * 2010-03-11 ms
 */
public function guaranteeFields($requiredFields, $data = null) {
    $guaranteedFields = array();
    foreach ($requiredFields as $column) {
        if (strpos($column, '.') !== false) {
            list($model, $column) = explode('.', $column, 2);
        } else {
            $model = $this->alias;
        }
        $guaranteedFields[$model][$column] = ''; # now field exists in any case!
    }
    if ($data === null) {
        return $guaranteedFields;
    }
    if (!empty($guaranteedFields)) {
        $data = Set::merge($guaranteedFields, $data);
    }
    return $data;
}

We can now make sure removing any form field will have no effect 🙂 I call it "enforced whitelisting".

Case 1: Using $this->Model->set($this->data) and $this->Model->save() – Important: note the NULL in save(), we may not pass the data a second time!

$this->User->set($this->data, null, array('gender', 'birthday', ...));
$this->User->save();

Case 2: Using directly $this->Model->guaranteeFields($fields, $this->data) and $this->Model->save($this->data)

$data = $this->User->guaranteeFields(array('gender', 'birthday', ...), $this->data);
$this->User->save($data);

Lets combine them

$this->data['User']['id'] = $id; // on edit
$this->User->set($this->data, null, array('gender', 'birthday', ...));
if ($this->User->save(null, true, array('id', 'gender', 'birthday', ...))) { # note the id (needed here)
    //OK
} else {
    //ERROR
}

Now thats protection 🙂
Only the fields that you want to are passed and all the fields that should be there are there. And the primary key is the one it is supposed to be. If your validation rules are correct, you got the perfect form handling, I guess.

Blacklisting

// put this in your app_model.php
public function blacklist($blackList = array()) {
    return array_diff(array_keys($this->schema()), $blackList);
}

Although not recommended for general usage, this might be useful for some rare occasions.
It returns all fields of the current database table except the ones you blacklisted.

Usage:

$this->User->save(null, true, $this->blacklist(array('birthday'));

If you want to pass non-existing fields to the model for validation you need to array_merge() them!

$this->User->save(null, true, array_merge(array('virtual_field'), $this->blacklist(array('birthday')));

Server-side and Client-side Validation

First one is what we just discussed. Client-side usually is JS or browser-specific validation. You should NEVER rely on client-side validation – it can easily be tricked. It is nice to have it – "additionally" in order to prevent unnecessary posts, but the real deal should be what your model validation returns.

SQL Injection

I always smile about other webprojects that are vulnerable to this attack. Its just hilarious. Thats why you use a framework. Because it takes care of the basic problems. And this is one of it.
There are only very few things to be aware of:
Always manually escape manual SQL queries!
CakePHP only takes care of the queries run by $this->save(), $this->saveAll() etc. So if you ever have to write your own SQL script, you will need to do that yourself.

The myth about "required" and "allowEmpty"

They seem to be the same, don’t they? But there is a huge difference between them (the cookbook does mention it here).
"required" is like my "Protection against missing fields", only hard-coded. These fields will validate false, if this field is not present on validation. It does not matter if the field is empty or not. The word "required" only applies to the database field, not its content.
"allowEmpty" is equal to "minLength 1" and ONLY validates this field if the field has been passed to the model. If not, it will not return false. You cannot make sure that a specific field is always filled out with this parameter alone.

If you want to make sure, that the field cannot be avoided AND has to be filled out, you need to combine both rules:

public $validate = array(
    'status' => array(
        'maxLength' => array( # this can be any label you want
            'required' => true, # careful!
            'allowEmpty' => false,
            'rule' => array('maxLength', 5),
            'message' => array('valErrMaxCharacters %s', 5), # a modification of mine to allow better i18n support
            'last' => true # important for more than one rule per field!!!
        ),
        ... # other rules
    ),
    ... # other fields
);

The "careful" stands for: this can get ugly really fast. I never use "required" hard-coded. As soon as you use ajax toggling of some fields or forms which only change a small part of the record, you will get validation errors because the required fields are not present (although you don’t even want to change them). You would need to unset() those rules manually. See the above "Protection against missing fields" for a cleaner approach.

Anyway, i hope this clears up things a little bit.

If you really feel that you need to use "required" you should only use it combined with ‘on’ => ‘create’, as well. In 99% of all cases you end up with never validating "edit" validations otherwise. That’t because you often save only a part of the data where you normally wouldn’t provide those required fields.
So as a guideline:

• Either don’t use required attribute at all and use my whitelisting approach from above
• Use required with "on create" IF (!) the field always has to be present for all create actions. For other actions use whitelisting and guaranteeFields

A few words to validation rules in general

You can add rules for "imaginary fields", as well. The fields don’t have to exist in the database model. This way you can easily use inputs which are validated separately and combined later on (eg: pwd and pwd_repeat resulting in the final sha1-hashed "password").
Always use "last"=>true for your rules. Unfortunately, the default value is still false here. "true" makes sure that after the first rule already returned false, the following ones are not checked as well (overhead and totally useless).
Also sort your rule array properly. The rules are checked from top to bottom, so it would make sense to put all complicated stuff at the bottom and first check on simple "notEmpty" or "email" rules. The most likely ones to fail at the beginning. Why should you check "isUnique" (database query!) before "notEmpty"? Why should you check an email address exists in the database if it is invalid in the first place?

Last words

Why didn’t I use Security Component for some of the above tasks? Well, it was a bitch to use in the first months. All the time some kind of black holes (white screen of death) without any indication what went wrong. Login/Logout, some other forms as well. With the first ajax forms it got even more messy and i deactivated it at last. Works quite well without it, if you protect the forms manually. With lots of ajax stuff going on you have to do that yourself, anyway…
And against certain attacks the Security Component won’t help anyway (PrimaryKey injection in edit forms etc). So at some point you will have to deal with it yourself. You have to decide to what point. But better sooner than later. If you adjust your bake templates before you start a new project it will save you lots and lots of time while having secure forms and procedures.

Note – 2012-06-02

For Cake2.x please use $this->request->data instead of $this->data in the controller scope.