Skip to content

Commit

Permalink
Fixed issue #19252: [security] Bad permission check when edit quota (#…
Browse files Browse the repository at this point in the history 
…3633)
  • Loading branch information
@Shnoulle
Shnoulle committed Nov 27, 2023
1 parent 10ba904 commit 1f69ac5
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 79 deletions.
143 changes: 70 additions & 73 deletions application/controllers/QuotasController.php
View file
Expand Up @@ -9,7 +9,7 @@ public function filters()
{
return array(
'accessControl', // perform access control for CRUD operations
'postOnly + deleteQuota', // we only allow deletion via POST request
'postOnly + deleteAnswer, deleteQuota, insertQuotaAnswer', // we only allow deletion via POST request
);
}

Expand All @@ -35,9 +35,8 @@ protected function beforeRender($view)
public function actionIndex($surveyid)
{
$surveyid = sanitize_int($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
if (!Permission::model()->hasSurveyPermission($surveyid, 'quotas')) {
throw new CHttpException(403, gT("You do not have permission on this survey."));
}
$oSurvey = Survey::model()->findByPk($surveyid);
$aData['surveyid'] = $oSurvey->sid;
Expand Down Expand Up @@ -105,9 +104,8 @@ public function actionIndex($surveyid)
public function actionQuickCSVReport($surveyid)
{
$surveyid = sanitize_int($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
if (!Permission::model()->hasSurveyPermission($surveyId, 'quotas')) {
throw new CHttpException(403, gT("You do not have permission on this survey."));
}
$oSurvey = Survey::model()->findByPk($surveyid);

Expand All @@ -134,11 +132,9 @@ public function actionQuickCSVReport($surveyid)
public function actionAddNewQuota($surveyid)
{
$surveyid = sanitize_int($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'create'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
if (!Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'create')) {
throw new CHttpException(403, gT("You do not have permission on this survey."));
}

Yii::app()->loadHelper('admin.htmleditor');

$oSurvey = Survey::model()->findByPk($surveyid);
Expand Down Expand Up @@ -170,8 +166,8 @@ public function actionAddNewQuota($surveyid)
$oQuota = new Quota();
$oQuota->sid = $oSurvey->primaryKey;
$quotaService = new \LimeSurvey\Models\Services\Quotas($oSurvey);
if (isset($_POST['Quota'])) {
$oQuota = $quotaService->saveNewQuota($_POST['Quota']);
if (App()->getRequest()->getPost('Quota')) {
$oQuota = $quotaService->saveNewQuota(App()->getRequest()->getPost('Quota'));
if (!$oQuota->getErrors()) {
Yii::app()->user->setFlash('success', gT("New quota saved"));
$this->redirect($this->createUrl("quotas/index/surveyid/$surveyid"));
Expand All @@ -191,24 +187,20 @@ public function actionAddNewQuota($surveyid)
}

/**
* @param $surveyid
* @return void
*/
public function actionEditQuota($surveyid)
public function actionEditQuota()
{
$surveyid = sanitize_int($surveyid);
$oSurvey = Survey::model()->findByPk($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'update'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
}

$quotaId = sanitize_int(Yii::app()->request->getQuery('quota_id'));
$oQuota = $this->getQuotaWithPermission($quotaId, 'update');
$surveyid = $oQuota->sid;
$oSurvey = Survey::model()->findByPk($surveyid);

/* @var Quota $oQuota */
$oQuota = Quota::model()->findByPk($quotaId);

if (isset($_POST['Quota'])) {
if (App()->getRequest()->getPost('Quota')) {
$quotaService = new \LimeSurvey\Models\Services\Quotas($oSurvey);
if ($quotaService->editQuota($oQuota, $_POST['Quota']) && !$oQuota->getErrors()) {
Yii::app()->user->setFlash('success', gT("Quota saved"));
Expand Down Expand Up @@ -257,45 +249,34 @@ public function actionEditQuota($surveyid)
*/
public function actionDeleteQuota()
{
$surveyid = sanitize_int(Yii::app()->request->getPost('surveyid'));
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'delete'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
}

$quotaId = Yii::app()->request->getPost('quota_id');
$oQuota = $this->getQuotaWithPermission($quotaId, 'delete');
$surveyid = $oQuota->sid;

Quota::model()->deleteByPk($quotaId);
QuotaLanguageSetting::model()->deleteAllByAttributes(array('quotals_quota_id' => $quotaId));
QuotaMember::model()->deleteAllByAttributes(array('quota_id' => $quotaId));

Yii::app()->user->setFlash('success', sprintf(gT("Quota with ID %s was deleted"), $quotaId));

$this->redirect($this->createUrl("quotas/index/surveyid/$surveyid"));
$this->redirect($this->createUrl(["quotas/index/",[ 'surveyid' => $surveyid]]));
}


/**
* @param int $surveyid
* @param string $sSubAction
* @return void
*/
public function actionNewAnswer(int $surveyid)
public function actionNewAnswer()
{
$surveyid = sanitize_int($surveyid);

$quotaId = Yii::app()->request->getParam('quota_id');
$quota = $this->getQuotaWithPermission($quotaId, 'delete');
$surveyid = $quota->sid;
$oSurvey = Survey::model()->findByPk($surveyid);
$aData['surveyid'] = $surveyid;
$sSubAction = Yii::app()->request->getParam('sSubaction', 'newanswer');

$sSubAction = Yii::app()->request->getParam('sSubaction');
if ($sSubAction === null) {
$sSubAction = 'newanswer';
}

if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'update'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
}
$renderView = array();
$quotaId = Yii::app()->request->getParam('quota_id');
$quota = Quota::model()->findByPk($quotaId);
$aData['oQuota'] = $quota;

Expand Down Expand Up @@ -356,27 +337,27 @@ public function actionNewAnswer(int $surveyid)
/**
* @return void
*/
public function actionInsertQuotaAnswer($surveyid)
public function actionInsertQuotaAnswer()
{
$surveyid = sanitize_int($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'update'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
}
$quota_qid = Yii::app()->request->getPost('quota_qid');
$quota_id = Yii::app()->request->getPost('quota_id');
$quota_anscode = Yii::app()->request->getPost('quota_anscode');
$oQuota = $this->getQuotaWithPermission($quota_id, 'update');
$surveyid = $oQuota->sid;

$oQuotaMembers = new QuotaMember('create'); // Trigger the 'create' rules
$oQuotaMembers->sid = $surveyid;
$oQuotaMembers->qid = Yii::app()->request->getPost('quota_qid');
$oQuotaMembers->quota_id = Yii::app()->request->getPost('quota_id');
$oQuotaMembers->code = Yii::app()->request->getPost('quota_anscode');
$oQuotaMembers->qid = $quota_qid;
$oQuotaMembers->quota_id = $quota_id;
$oQuotaMembers->code = $quota_anscode;
if ($oQuotaMembers->save()) {
if (!empty($_POST['createanother'])) {
if (App()->getRequest()->getPost('createanother')) {
$this->redirect($this->createUrl(
'quotas/newAnswer',
[
'surveyid' => $surveyid,
'sSubAction' => 'newanswer',
'quota_id' => Yii::app()->request->getPost('quota_id')
'quota_id' => $quota_id
]
));
} else {
Expand All @@ -389,7 +370,7 @@ public function actionInsertQuotaAnswer($surveyid)
[
'surveyid' => $surveyid,
'sSubAction' => 'newanswer',
'quota_id' => Yii::app()->request->getPost('quota_id')
'quota_id' => $quota_id
]
));
}
Expand All @@ -398,21 +379,17 @@ public function actionInsertQuotaAnswer($surveyid)
/**
* @return void
*/
public function actionDeleteAnswer($surveyid)
public function actionDeleteAnswer()
{
$surveyid = sanitize_int($surveyid);
if (!(Permission::model()->hasSurveyPermission($surveyid, 'quotas', 'delete'))) {
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
$id = App()->request->getPost('quota_member_id');
$quotaMember = QuotaMember::model()->findByPk($id);
if (empty($quotaMember)) {
throw new CHttpException(404, gT("Quota member not found."));
}

QuotaMember::model()->deleteAllByAttributes(array(
'id' => Yii::app()->request->getPost('quota_member_id'),
'qid' => Yii::app()->request->getPost('quota_qid'),
'code' => Yii::app()->request->getPost('quota_anscode'),
));

$this->redirect($this->createUrl('/quotas/index', ['surveyid' => $surveyid]));
$oQuota = $this->getQuotaWithPermission($quotaMember->quota_id, 'delete');
$surveyid = $oQuota->sid;
$quotaMember->delete();
$this->redirect($this->createUrl('/quotas/index', ['surveyid' => $oQuota->sid]));
}

/**
Expand All @@ -427,19 +404,39 @@ public function actionMassiveAction($action, $surveyid)
if ($quotaService->checkActionPermissions($action)) {
$sItems = Yii::app()->request->getPost('sItems', '');
$aQuotaIds = json_decode($sItems);
if (isset($_POST['QuotaLanguageSetting'])) {
$errors = $quotaService->multipleItemsAction($aQuotaIds, $action, $_POST['QuotaLanguageSetting']);
} else {
$errors = $quotaService->multipleItemsAction($aQuotaIds, $action);
}
$errors = $quotaService->multipleItemsAction(
$aQuotaIds,
$action,
Yii::app()->request->getPost('QuotaLanguageSetting', [])
);
if (empty($errors)) {
eT("OK!");
} else {
eT("Error!");
}
} else {
/* 403 error ? */
Yii::app()->user->setFlash('error', gT("Access denied."));
$this->redirect(Yii::app()->request->urlReferrer);
}
}

/**
* Get a quota after check exist and permission using permission on survey
* @param integer $quotaId
* @param string $sPermission to check (on survey quotas)
* throw Exception
* @return \Quota
*/
private function getQuotaWithPermission($quotaId, $sPermission = 'read')
{
$oQuota = Quota::model()->findByPk($quotaId);
if (empty($oQuota)) {
throw new CHttpException(404, gT("Quota not found."));
}
if (!Permission::model()->hasSurveyPermission($oQuota->sid, 'quotas', $sPermission)) {
throw new CHttpException(403, gT("You do not have permission on this quota."));
}
return $oQuota;
}
}
18 changes: 12 additions & 6 deletions application/models/services/Quotas.php
View file
Expand Up @@ -201,9 +201,12 @@ public function saveNewQuota(array $quotaParams): \Quota
{
$oQuota = new \Quota();
$oQuota->sid = $this->survey->sid;
/* new quota : remove pk */
unset($quotaParams['id']);
$oQuota->attributes = $quotaParams;
if ($oQuota->save()) {
foreach ($_POST['QuotaLanguageSetting'] as $language => $settingAttributes) {
$postQuotaLanguageSettings = (array) App()->getRequest()->getPost('QuotaLanguageSetting');
foreach ($postQuotaLanguageSettings as $language => $settingAttributes) {
$oQuotaLanguageSetting = new \QuotaLanguageSetting();
$oQuotaLanguageSetting->attributes = $settingAttributes;
$oQuotaLanguageSetting->quotals_quota_id = $oQuota->primaryKey;
Expand Down Expand Up @@ -305,10 +308,10 @@ public function allAnswersSelected(\Question $oQuestion, array $aQuestionAnswers

/**
*
* @param $aQuotaIds
* @param $action
* @param $languageSettings
* @return null | array errors or null if no errors
* @param integer[] $aQuotaIds
* @param string $action
* @param null|array $languageSettings
* @return null|array errors or null if no errors
* @throws \CDbException
*/
public function multipleItemsAction($aQuotaIds, $action, $languageSettings = [])
Expand All @@ -317,6 +320,9 @@ public function multipleItemsAction($aQuotaIds, $action, $languageSettings = [])
foreach ($aQuotaIds as $iQuotaId) {
/** @var \Quota $oQuota */
$oQuota = \Quota::model()->findByPk($iQuotaId);
if (empty($oQuota) || $oQuota->sid != $this->survey->sid) {
$errors [] = gT("Invalid quota id");
}
switch ($action) {
case 'activate':
case 'deactivate':
Expand Down Expand Up @@ -345,7 +351,7 @@ public function multipleItemsAction($aQuotaIds, $action, $languageSettings = [])
}
break;
default:
$errors [] = 'No valid action';
$errors [] = gT('No valid action');
}
}

Expand Down

0 comments on commit 1f69ac5

Please sign in to comment.