Data security and $model->load()

Doing a code review. We have a lot of examples of this in a controller:

public function actionSomeCreate() 
{
	$model = SomeForm();
	
	$model->user_id = Yii::$app->user->id;
	
	if ($model->load(Yii::$app->request->post()) && $model->validate()) {
		$model->save();
	}
	
	...
}

But isn’t this risky? Is it possible for somebody on the client side to add user_id to the POST values and override what is set in $model->user_id? A lot of our validation rules rely on $model->user being set however, so that needs to happen before the validate(). Is this not a concern? Is it prevented in some way?

The other option is:

if ($model->load(Yii::$app->request->post())) {
	$model->user_id = Yii::$app->user->id;
	
	if ($model->validate()) {
		$model->save();
	}
}

What say ye?

First of all - if there is no validation rule for user_id it won’t be possible to load the value with load() method.
Second - if you need to allow setting the user_id by the client and you add validation rule for it but still want in some cases for this to be fixed unchangeable value you can prepare special validation rule or super-simple - just put $model->user_id = Yii::$app->user->id; before $model->save(); so it will overwrite anything provided by the client.

BTW. don’t do just this - $model->save();. It can fail as well, always check the result of save(). Also - if you call validate() just before save() you can set first argument of save() to false so you will not trigger validation twice.

2 Likes

Gosh, actually, I was skimming our code to prep for the review and asked this question. In hindsight, I remember that yii2 uses the ‘safe’ rule to mark an attribute as safe for update by load(). So that answers that!

As far as $model->save(), you are correct. We use a function called saveOrError() that does a final validate() and throw() as needed as a last-ditch error to catch issues. That doesn’t include the normal validation and error catching in the backend code. Great point though!