RSS
 

Posts Tagged ‘XSS’

ResetBehavior and HazardableBehavior

29 Oct

ResetBehavior

Reset is a new behavior I recently had to write to update some geocoded entries as well as records with processed titles/slugs – via beforeSave() – that had to be re-saved with a corrected processing method. Basically, it is a batch update that works even with huge tables as it processes the data in steps.

Example: Resetting slugs

Either via shell or via controller action you can trigger an update of all slugs that have been generated via Tools plugin Slugged behavior:

// First we need to re-load the Slugged behavior to enable "overwrite" mode
$this->Post->Behaviors->load('Tools.Slugged', array('label' => 'title', 'overwite' => true));
// Load the Reset behavior with only the title and slug field to read and modify.
$this->Post->Behaviors->load('Tools.Reset', array('fields' => array('title', 'slug')));
$res = $this->Post->resetRecords();
// flash message with number of records modified in $res

You should make a shell command for this and execute this migration code once on deploy of the modified code or SQL schema.
If you are not using CLI, make sure you set the time limit in your controller action accordingly (HOUR for example).

Example: Retrigger/Init geocoding

If you got records that now need to be geocoded, you probably added a lat and lng field (decimal 6,2 etc) to your table.
You probably also attached the Tools plugin Geocoder behavior to this model.
In order to quickly geocode you just have to use Reset:

$this->Post->Behaviors->load('Tools.Reset', array('fields' => array('address', 'lat', 'lng'), 'timeout' => 3));
$res = $this->Post->resetRecords();

Since all lat/lng fields are still null it will geocode the records and populate those fields. It will skip already geocoded ones. If you
want to skip those completely (not even read them), just set the scope to 'NOT' => array('lat' => null) etc.

Note that in this case we also use a timeout to avoid getting a penalty by Google for geocoding too many records per minute.

Advanced example: Resetting composite cache field

In this case we added a new cache field to our messages in order to make the search faster with >> 100000 records. The data was containing
all the info we needed – in serialized format.
We needed a callback here as there was some logic involved. So we simply made a shell containing both callback method and shell command:

$this->Message->Behaviors->load('Tools.Reset', array(
	'fields' => array('data'), 'updateFields' => array('guest_name'),
	'scope' => array('data LIKE' => '{%'), 'callback' => 'UpdateShell::prepMessage'));
$res = $this->Message->resetRecords();
$this->out('Done: ' . $res);

The callback method (in this case just statically, as we didnt want to mess with the model itself):

public static function prepMessage(array $row) {
	if (empty($row['Message']['data_array']['GUEST_FIRST_NAME'])) {
		return array();
	}
	$row['Message']['guest_name'] = $row['Message']['data_array']['GUEST_FIRST_NAME'] . ' ' . $row['Message']['data_array']['GUEST_LAST_NAME'];
	return $row;
}

See the test cases for more ways to use callbacks – including adjusting the updateFields list.

So as you can see, everything that involves a complete "re-save" including triggering of important callbacks (in model and behaviors) of all or most records can leverage this behavior in a DRY, quick and reusable way.

HazardableBehavior

This behavior is a very useful tool to test vulnerability against XSS or unescaped html output (especially accidental one).
The basic idea is to test all views that output data from varchar or text fields for proper escaping. Even if it not user input, it is still vital to properly escape.
Admin input can have chars like <, > etc in there, as well. Without the use of h() it can destroy the layout or worse. So it is always a good idea to cover all
views.

Just attach it temporarily (!) to any of your models and quickly fill your table with hazardous strings. Those strings can potentially end up there via Form input, of course.
This just automates it.
Then you can browse your site and see if an alert or other strange behavior occurs. This tells you that you forgot to use h() or other measures to secure your output properly.

You can also apply this behavior globally to overwrite all strings in the find result.
This way you don’t need to modify the database. On output it will just inject the hazardous strings and you can browse your website just as if they were actually stored in your db.
Just add it to some models or even the AppModel (temporarily!) as $actsAs = array('Tools.Hazardable' => array('replaceFind' => true)).
A known limitation of Cake behaviors in 2.x, though, is, that this would only apply for first-level records (not related data). So it is usually better to insert some hazardous strings into all your tables and make your tests then as closely to the reality as possible.

You can use skipFields to blacklist certain stringish fields from being overwritten and populated with hazardous strings.

Note: In 3.x the behavior callback issue regarding non-primary records will be solved πŸ™‚ I am really looking forward to that.

CakePHP 3

This article is 2.x only.
For CakePHP 3 please see the 3.0 Tools Plugin documentation on the Reset behavior.

 
No Comments

Posted in CakePHP

 

Saving Model Data and Security

21 Sep

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.

 
14 Comments

Posted in CakePHP