Fixed issue: [security] Various major & minor security issues discove…

…red by automated security tools
LimeSurvey · Oct 18, 2022 · 008134d · Shnoulle · Apr 25, 2023 · Shnoulle
1 parent 2d3ae82
commit 008134d
Show file tree

Hide file tree

Showing 29 changed files with 227 additions and 180 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -3,6 +3,7 @@ on:
   push:
     branches:
       - master
+      - sonarcubefixes
   pull_request:
     types: [opened, synchronize, reopened]
 jobs:

diff --git a/admin/admin.php b/admin/admin.php
@@ -1,3 +1,3 @@
 <?php
 
-include 'index.php';
+include_once 'index.php';
diff --git a/application/controllers/QuestionAdministrationController.php b/application/controllers/QuestionAdministrationController.php
@@ -1066,8 +1066,8 @@ public function actionImportView($surveyid, $groupid = null)
      */
     public function actionImport()
     {
-        $iSurveyID = App()->request->getPost('sid', 0);
-        $gid = App()->request->getPost('gid', 0);
+        $iSurveyID = (int) App()->request->getPost('sid', 0);
+        $gid = (int) App()->request->getPost('gid', 0);
 
         $jumptoquestion = (bool)App()->request->getPost('jumptoquestion', 1);
 

diff --git a/application/controllers/SurveyAdministrationController.php b/application/controllers/SurveyAdministrationController.php
@@ -508,7 +508,7 @@ public function actionInsert()
                 $iNewGroupID = $this->createSampleGroup($iNewSurveyid);
                 $iNewQuestionID = $this->createSampleQuestion($iNewSurveyid, $iNewGroupID);
 
-                Yii::app()->setFlashMessage(gT("Your new survey was created. 
+                Yii::app()->setFlashMessage(gT("Your new survey was created.
                 We also created a first question group and an example question for you."), 'info');
                 $redirecturl = $this->getSurveyAndSidemenueDirectionURL(
                     $iNewSurveyid,
@@ -566,7 +566,7 @@ public function actionFakebrowser()
      */
     public function actionImportsurveyresources()
     {
-        $iSurveyID = Yii::app()->request->getPost('surveyid');
+        $iSurveyID = sanitize_int(Yii::app()->request->getPost('surveyid'));
         if (!Permission::model()->hasSurveyPermission($iSurveyID, 'surveycontent', 'update')) {
             Yii::app()->user->setFlash('error', gT("Access denied"));
             $this->redirect(Yii::app()->request->urlReferrer);
@@ -1288,7 +1288,7 @@ public function actionUploadimagefile()
         $uploadValidator = new LimeSurvey\Models\Services\UploadValidator();
         $uploadValidator->renderJsonOnError('file', $debug);
 
-        $iSurveyID = Yii::app()->request->getPost('surveyid');
+        $iSurveyID = (int) Yii::app()->request->getPost('surveyid');
         $success = false;
         $debug = [];
         if (!Permission::model()->hasSurveyPermission($iSurveyID, 'surveycontent', 'update')) {
@@ -2983,7 +2983,7 @@ protected function getTextEditData($survey)
             "window.TextEditData = {
                 connectorBaseUrl: '" . Yii::app()->getController()->createUrl('surveyAdministration/') . "',
                 isNewSurvey: " . ($survey->getIsNewRecord() ? "true" : "false") . ",
-                sid: $survey->sid, 
+                sid: $survey->sid,
                 i10N: {
                     'Survey title' : '" . gT('Survey title') . "',
                     'Date format' : '" . gT('Date format') . "',

diff --git a/application/controllers/UploaderController.php b/application/controllers/UploaderController.php
@@ -56,23 +56,23 @@ public function run($actionID)
             // If one seems to be a hack: Bad request
             throw new CHttpException(400); // See for debug > 1
         }
-        if ($sFileGetContent) {
-            if (substr($sFileGetContent, 0, 6) == 'futmp_') {
+        if ($sFileGetContentFiltered) {
+            if (substr($sFileGetContentFiltered, 0, 6) == 'futmp_') {
                 $sFileDir = $tempdir . '/upload/';
-            } elseif (substr($sFileGetContent, 0, 3) == 'fu_') {
+            } elseif (substr($sFileGetContentFiltered, 0, 3) == 'fu_') {
                 // Need to validate $_SESSION['srid'], and this file is from this srid !
                 $sFileDir = "{$uploaddir}/surveys/{$surveyid}/files/";
             } else {
                 throw new CHttpException(400); // See for debug > 1
             }
-            if (is_file($sFileDir . $sFileGetContent)) {
+            if (is_file($sFileDir . $sFileGetContentFiltered)) {
                 // Validate file before else 500 error by getMimeType
-                $mimeType = LSFileHelper::getMimeType($sFileDir . $sFileGetContent, null, false);
+                $mimeType = LSFileHelper::getMimeType($sFileDir . $sFileGetContentFiltered, null, false);
                 if (is_null($mimeType)) {
                     $mimeType = "application/octet-stream"; // Can not really get content if not image
                 }
                 header('Content-Type: ' . $mimeType);
-                readfile($sFileDir . $sFileGetContent);
+                readfile($sFileDir . $sFileGetContentFiltered);
                 Yii::app()->end();
             } else {
                 Yii::app()->end();
@@ -109,8 +109,8 @@ public function run($actionID)
             }
             //var_dump($sFileDir.$sFilename);
             // Return some json to do a beautiful text
-            if (@unlink($sFileDir . $sFileName)) {
-                echo sprintf(gT('File %s deleted'), $sOriginalFileName);
+            if (@unlink($sFileDir . $sFileNameFiltered)) {
+                echo sprintf(gT('File %s deleted'), CHtml::encode($sOriginalFileName));
             } else {
                 echo gT('Oops, There was an error deleting the file');
             }

diff --git a/application/controllers/admin/ConditionsAction.php b/application/controllers/admin/ConditionsAction.php
@@ -1072,7 +1072,7 @@ protected function copyConditions(array $args)
         $copyconditionsfrom = returnGlobal('copyconditionsfrom');
         $copyconditionsto = returnGlobal('copyconditionsto');
         if (isset($copyconditionsto) && is_array($copyconditionsto) && isset($copyconditionsfrom) && is_array($copyconditionsfrom)) {
-            //Get the conditions we are going to copy
+            //Get the conditions we are going to copy and quote them properly
             foreach ($copyconditionsfrom as &$entry) {
                 $entry = Yii::app()->db->quoteValue($entry);
             }
@@ -1085,11 +1085,11 @@ protected function copyConditions(array $args)
 
             foreach ($result->readAll() as $row) {
                 $proformaconditions[] = array(
-                    "scenario"        =>    $row['scenario'],
-                    "cqid"            =>    $row['cqid'],
+                    "scenario"      =>    $row['scenario'],
+                    "cqid"          =>    $row['cqid'],
                     "cfieldname"    =>    $row['cfieldname'],
                     "method"        =>    $row['method'],
-                    "value"            =>    $row['value']
+                    "value"         =>    $row['value']
                 );
             } // while
 
@@ -1100,7 +1100,7 @@ protected function copyConditions(array $args)
 
                     //First lets make sure there isn't already an exact replica of this condition
                     $conditions_data = array(
-                        'qid'        => $newqid,
+                        'qid'        => (int) $newqid,
                         'scenario'   => $pfc['scenario'],
                         'cqid'       => $pfc['cqid'],
                         'cfieldname' => $pfc['cfieldname'],

diff --git a/application/controllers/admin/Export.php b/application/controllers/admin/Export.php
@@ -387,7 +387,7 @@ public function exportspss()
             // Default to 2 (16 and up)
             Yii::app()->session['spssversion'] = 2;
         }
-        $spssver = Yii::app()->request->getParam('spssver', Yii::app()->session['spssversion']);
+        $spssver = CHtml::encode(Yii::app()->request->getParam('spssver', Yii::app()->session['spssversion']));
         Yii::app()->session['spssversion'] = $spssver;
 
         $length_varlabel = '231'; // Set the max text length of Variable Labels
@@ -1285,7 +1285,9 @@ public function quexml(int $iSurveyID)
                 $quexmlpdf->$method(Yii::app()->request->getPost($s));
             }
 
-            $lang = Yii::app()->request->getPost('save_language');
+            $lang = sanitize_languagecode(
+                Yii::app()->request->getPost('save_language')
+            );
 
             // Setting the selected language for printout
             App()->setLanguage($lang);

diff --git a/application/controllers/admin/Expressions.php b/application/controllers/admin/Expressions.php
@@ -84,9 +84,7 @@ public function survey_logic_file()
     {
         $aData = array();
 
-        $sid = Yii::app()->request->getParam('sid', 0, 'integer');
-        $surveyid = Yii::app()->request->getParam('surveyid', $sid, 'integer');
-
+        $sid = (int) Yii::app()->request->getParam('sid', 0);
         $hasSurveyContentReadPermission = Permission::model()->hasSurveyPermission($sid, 'surveycontent', 'read');
         if (!$hasSurveyContentReadPermission) {
             $message['title'] = gT('Access denied!');

diff --git a/application/controllers/admin/Labels.php b/application/controllers/admin/Labels.php
@@ -46,7 +46,7 @@ public function importlabelresources()
             Yii::app()->session['flashmessage'] = gT('Access denied!');
             $this->getController()->redirect(App()->createUrl("/admin"));
         }
-        $lid = returnGlobal('lid');
+        $lid = (int) Yii::app()->request->getParam('lid');
         if (!empty($lid)) {
             if (Yii::app()->getConfig('demoMode')) {
                 $this->getController()->error(gT("Demo mode only: Uploading files is disabled in this system."), $this->getController()->createUrl("admin/labels/sa/view/lid/{$lid}"));

diff --git a/application/controllers/admin/ParticipantsAction.php b/application/controllers/admin/ParticipantsAction.php
@@ -1454,7 +1454,10 @@ public function changeAttributeEncrypted()
                 }
             } else {
                 // custom participant attributes
-                $oAttributes = ParticipantAttribute::model()->findAll("attribute_id=:attribute_id", array("attribute_id" => $attributeId));
+                $oAttributes = ParticipantAttribute::model()->findAll(
+                    'attribute_id = :attribute_id',
+                    array(':attribute_id' => $attributeId)
+                );
                 foreach ($oAttributes as $attribute) {
                     $aUpdateData = array();
                     if ($sEncryptedBeforeChange == 'Y' && $sEncryptedAfterChange == 'N') {
@@ -1463,7 +1466,15 @@ public function changeAttributeEncrypted()
                         $aUpdateData['value'] = LSActiveRecord::encryptSingle($attribute->value);
                     }
                     if (!empty($aUpdateData) && $aUpdateData['value'] !== null) {
-                        $oDB->createCommand()->update('{{participant_attribute}}', $aUpdateData, "attribute_id='" . $attributeId . "' AND participant_id = '" . $attribute->participant_id . "'");
+                        $oDB->createCommand()->update(
+                            '{{participant_attribute}}',
+                            $aUpdateData,
+                            'attribute_id = :attribute_id AND participant_id = :participant_id',
+                            array(
+                                ':attribute_id' => $attributeId,
+                                ':participant_id' => $attribute->participant_id
+                            )
+                        );
                     }
                 }
             }
@@ -1607,7 +1618,15 @@ public function editAttributeName()
                     $aUpdateData['value'] = LSActiveRecord::encryptSingle($attribute->value);
                 }
                 if (!empty($aUpdateData)) {
-                    $oDB->createCommand()->update('{{participant_attribute}}', $aUpdateData, "attribute_id='" . $iAttributeId . "' AND participant_id = '" . $attribute->participant_id . "'");
+                    $oDB->createCommand()->update(
+                        '{{participant_attribute}}',
+                        $aUpdateData,
+                        "attribute_id = :attribute_id AND participant_id = :participant_id",
+                        array(
+                            ':attribute_id' => $iAttributeId,
+                            ':participant_id' => $attribute->participant_id
+                        )
+                    );
                 }
             }
 
@@ -1670,7 +1689,7 @@ public function deleteLanguageFromAttribute()
      */
     public function deleteSingleAttribute()
     {
-        $attribute_id = Yii::app()->request->getPost('attribute_id');
+        $attribute_id = (int) Yii::app()->request->getPost('attribute_id');
         ParticipantAttributeName::model()->delAttribute($attribute_id);
         $this->ajaxHelper::outputSuccess(gT("Attribute successfully deleted"));
     }
@@ -1689,6 +1708,7 @@ public function deleteAttributes()
 
         $request = Yii::app()->request;
         $attributeIds = json_decode($request->getPost('sItems'));
+        $attributeIds = array_map('sanitize_int', $attributeIds);
 
         $deletedAttributes = 0;
 
@@ -1933,8 +1953,8 @@ public function saveAttribute()
      */
     public function delAttributeValues()
     {
-        $iAttributeId = Yii::app()->request->getQuery('aid');
-        $iValueId = Yii::app()->request->getQuery('vid');
+        $iAttributeId = (int) Yii::app()->request->getQuery('aid');
+        $iValueId = (int) Yii::app()->request->getQuery('vid');
         ParticipantAttributeName::model()->delAttributeValues($iAttributeId, $iValueId);
         Yii::app()->getController()->redirect(array('/admin/participants/sa/viewAttribute/aid/' . $iAttributeId));
     }
@@ -2471,7 +2491,7 @@ public function addToCentral()
         $overwriteman = Yii::app()->request->getPost('overwriteman', false);
         $createautomap = Yii::app()->request->getPost('createautomap');
 
-        $response = Participant::model()->copyToCentral(Yii::app()->request->getPost('surveyid'), $newarr, $mapped, $overwriteauto, $overwriteman, $createautomap);
+        $response = Participant::model()->copyToCentral((int) Yii::app()->request->getPost('surveyid'), $newarr, $mapped, $overwriteauto, $overwriteman, $createautomap);
 
         echo "<p>";
         printf(gT("%s participants have been copied to the central participants table"), "<span class='badge alert-success'>" . $response['success'] . "</span>&nbsp;");
@@ -2502,7 +2522,7 @@ public function addToTokenattmap()
         $participantIdsString = Yii::app()->request->getPost('participant_id'); // TODO: This is a comma separated string of ids
         $participantIds = explode(",", $participantIdsString);
 
-        $surveyId = Yii::app()->request->getPost('surveyid');
+        $surveyId = (int)Yii::app()->request->getPost('surveyid');
 
         /**
          * mapped can take values like

diff --git a/application/controllers/admin/PluginManagerController.php b/application/controllers/admin/PluginManagerController.php
@@ -440,7 +440,7 @@ public function installPluginFromFile()
         $this->checkUpdatePermission();
 
         $request = Yii::app()->request;
-        $pluginName = $request->getPost('pluginName');
+        $pluginName = sanitize_alphanumeric($request->getPost('pluginName'));
 
         $pluginManager = App()->getPluginManager();
         $pluginInfo = $pluginManager->getPluginInfo($pluginName);

diff --git a/application/controllers/admin/Themes.php b/application/controllers/admin/Themes.php
@@ -638,8 +638,8 @@ public function templaterename()
         if (Permission::model()->hasGlobalPermission('templates', 'update')) {
             if (returnGlobal('action') == "templaterename" && returnGlobal('newname') && returnGlobal('copydir')) {
                 $sNewName = sanitize_dirname(returnGlobal('newname'));
-                $sNewDirectoryPath = Yii::app()->getConfig('userthemerootdir') . "/" . $sNewName;
-                $sOldDirectoryPath = Yii::app()->getConfig('userthemerootdir') . "/" . returnGlobal('copydir');
+                $sNewDirectoryPath = sanitize_dirname(Yii::app()->getConfig('userthemerootdir') . "/" . $sNewName);
+                $sOldDirectoryPath = sanitize_dirname(Yii::app()->getConfig('userthemerootdir') . "/" . returnGlobal('copydir'));
 
                 if (Template::isStandardTemplate(returnGlobal('newname'))) {
                     Yii::app()->user->setFlash('error', sprintf(gT("Template could not be renamed to '%s'."), $sNewName) . " " . gT("This name is reserved for standard template."));
@@ -883,7 +883,7 @@ public function templatesavechanges()
                         $oEditedTemplate->extendsFile($relativePathEditfile);
                     }
 
-                    $savefilename = $oEditedTemplate->extendsFile($relativePathEditfile);
+                    $savefilename = $oEditedTemplate->extendsFile($relativePathEditfile, $relativePathEditfile);
 
                     if (is_writable($savefilename)) {
                         if (!$handle = fopen($savefilename, 'w')) {

diff --git a/application/controllers/survey/index.php b/application/controllers/survey/index.php
@@ -30,8 +30,8 @@ public function action()
 
         $this->loadRequiredHelpersAndLibraries();
         $param       = $this->getParameters(func_get_args(), $_POST);
-        $surveyid    = $param['sid'];
-        $thisstep    = $param['thisstep'];
+        $surveyid    = (int) $param['sid'];
+        $thisstep    = (int) $param['thisstep'];
         $move        = getMove();
 
         /* Newtest must be done bedore all other action */