$_Get['id'] Security In Accessrules

Yii 1.1 General Discussion
1

So I have the following code in Users Controller:


public function accessRules()

	{

                        array('allow',

                                'actions'=>array('update', 'delete', 'view', 'statusChange'),

                                'users'=>array('@'),

                                'expression'=>'Users::model()->userManagementPermissions($_GET["id"])',

                        ),

		);

	}

And here is the function in the Users model as used in the above expression:


public function userManagementPermissions($id)

        {

                if(Yii::app()->user->user_permissions == 3){

                    return true;

                }

                else if (Yii::app()->user->user_permissions == 2){

                    //gettin the user_client_id value from the logged CompanyAdmin

                    $cAdminClient = $this->getClientId(Yii::app()->user->id);


                    //getting the user_client_id from the user being modified. Super Admins excluded.

                    $userClient = Yii::app()->db->createCommand()

                        ->select('user_client_id')

                        ->from('users')

                        ->where('user_id=:id', array(':id'=>$id))

                        ->andWhere('user_permisions<>3')

                        ->queryRow();


                    if($userClient['user_client_id'] == $cAdminClient)

                        return true;    //users belong to the same group, permissions granted

                    else

                        return false;

                } else {    //user permission level is 1

                    return false;

                }

        }

Now this code works perfectly. However, there is a security issue.

For example, if the URL is like this: /index.php?r=users/update&id=55 , this would work.

However, if the logged user cuts the URL and does something like this: /index.php?r=users/update , then I get the following error:

[b]PHP notice

Undefined index: id [/b]

This is because $_GET[‘id’] is not defined in the URL and cannot be used in the expression:


'expression'=>'Users::model()->userManagementPermissions($_GET["id"])'

How can I solve this?

Thank you in advance!

2

I think you may use


$_REQUEST['id']
3

Hi

use


'expression'=>'Users::model()->userManagementPermissions(Yii::app()->request()->getQuery("id"))',
4

Also if you want extra security or another handle then


public function userManagementPermissions($id=null) {

if ($id===null)

  throw new CHttpException(403, 'Invalid request');

...

...

In this way you could display the apropriate message you want

5

This solved my problem. But it should be


Yii::app()->request

instead of


Yii::app()->request()

.

Thank you for your answers guys.

6

Yes, Yii has internal mechanism that permits you to use methods like variable. for example


Yii::app()->request->getQuery("id")

is equivalent with


Yii::app()->getRequest()->getQuery("id")

So, You are right! thanks for your voting, you requite :)

7

Another potential error…

If someone were to edit the url and rather than omitting the id, they changed it instead to a value that doesn’t exist, like: /index.php?r=users/update&id=555555555555555

then the line below is going to give you an error because a row was never returned from the query:


if($userClient['user_client_id'] == $cAdminClient)

You need to check that $userClient has data or the key ‘user_client_id’ won’t exist.


if($userClient && $userClient['user_client_id'] == $cAdminClient)

or


if(isset($userClient['user_client_id']) && $userClient['user_client_id'] == $cAdminClient)

Also, do you need the querystring at all?

You said the code is in your users model, so when updating etc, shouldn’t ‘user_id’ ($id) be available as $this->user_id ??