Fixed issue #19038: [security] Privilege escalation bug to creation s…

…urvey-group with others group as parent (#3423) Co-authored-by: lapiudevgit <devgit@lapiu.biz>
LimeSurvey · Oct 9, 2023 · acd3476 · acd3476
1 parent ffb66e3
commit acd3476
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 1 deletion.
diff --git a/application/controllers/admin/SurveysGroupsController.php b/application/controllers/admin/SurveysGroupsController.php
@@ -66,6 +66,13 @@ public function create()
                 $this->getController()->redirect(
                     App()->createUrl("admin/surveysgroups/sa/update", array('id' => $model->gsid, '#' => 'settingsForThisGroup'))
                 );
+            } else {
+                $errors = $service->getMessages('error');
+                if (!empty($errors)) {
+                    foreach ($errors as $error) {
+                        Yii::app()->setFlashMessage($error->getMessage(), 'error');
+                    }
+                }
             }
         } else {
             $model->name = SurveysGroups::getNewCode();

diff --git a/application/models/SurveysGroups.php b/application/models/SurveysGroups.php
@@ -66,6 +66,7 @@ public function rules()
             array('title', 'length', 'max' => 100),
             array('alwaysavailable', 'boolean'),
             array('description, created, modified', 'safe'),
+            array('parent_id', 'in', 'range' => array_keys(self::getSurveyGroupsList()), 'allowEmpty' => true, 'message' => gT("You are not allowed to set this group as parent")),
             // The following rule is used by search().
             // @todo Please remove those attributes that should not be searched.
             array('gsid, name, title, description, owner_id, parent_id, created, modified, created_by', 'safe', 'on' => 'search'),

diff --git a/application/models/services/SurveysGroupCreator.php b/application/models/services/SurveysGroupCreator.php
@@ -2,6 +2,8 @@
 
 namespace LimeSurvey\Models\Services;
 
+use CHtml;
+use LimeSurvey\Datavalueobjects\TypedMessage;
 use LSHttpRequest;
 use LSWebUser;
 use SurveysGroups;
@@ -25,6 +27,9 @@ class SurveysGroupCreator
     /** @var SurveysGroupsettings */
     private $surveysGroupsettings;
 
+    /** @var TypedMessage[] an array of messages providing extra details */
+    private $messages = [];
+
     /**
      * @param LSHttpRequest $request
      * @param LSWebUser $user
@@ -58,9 +63,35 @@ public function save()
             $this->surveysGroupsettings->gsid = $this->surveysGroup->gsid;
             $this->surveysGroupsettings->setToInherit();
 
-            return $this->surveysGroupsettings->save();
+            if ($this->surveysGroupsettings->save()) {
+                return true;
+            } else {
+                $this->messages[] = new TypedMessage(CHtml::errorSummary($this->surveysGroupsettings), 'error');
+                return false;
+            }
         } else {
+            $this->messages[] = new TypedMessage(CHtml::errorSummary($this->surveysGroup), 'error');
             return false;
         }
     }
+
+    /**
+     * Returns the messages of the given type, or all messages if
+     * no type is specified.
+     * @param string|null $type
+     * @return TypedMessage[]
+     */
+    public function getMessages($type = null)
+    {
+        if (empty($type)) {
+            return $this->messages;
+        }
+        $messages = [];
+        foreach ($this->messages as $message) {
+            if ($message->getType() === $type) {
+                $messages[] = $message;
+            }
+        }
+        return $messages;
+    }
 }