Merge pull request #2821 from chamilo/fix-unserialize

ywarnier · web-flow · commit ba97eda117ae · 2019-03-01T12:33:05.000-05:00
Improve `unserialize` security
diff --git a/composer.json b/composer.json
@@ -112,7 +112,8 @@
         "knplabs/knp-components": "~1.3",
         "guzzlehttp/guzzle": "~6.0",
         "onelogin/php-saml": "^3.0",
-        "symfony/dom-crawler": "~3.4"
+        "symfony/dom-crawler": "~3.4",
+        "brumann/polyfill-unserialize": "^1.0"
     },
     "require-dev": {
         "behat/behat": "@stable",
diff --git a/main/admin/career_diagram.php b/main/admin/career_diagram.php
@@ -14,6 +14,8 @@
 ALTER TABLE extra_field_values modify column value longtext null;
 */
 
+use Fhaculty\Graph\Graph;
+
 $cidReset = true;
 require_once __DIR__.'/../inc/global.inc.php';
 
@@ -106,7 +108,8 @@
 $tpl = new Template(get_lang('Diagram'));
 $html = Display::page_subheader2($careerInfo['name'].$urlToString);
 if (!empty($item) && isset($item['value']) && !empty($item['value'])) {
-    $graph = unserialize($item['value']);
+    /** @var Graph $graph */
+    $graph = api_unserialize_content('career', $item['value']);
     $html .= Career::renderDiagramByColumn($graph, $tpl);
 } else {
     Display::addFlash(
diff --git a/main/admin/gradebook_list.php b/main/admin/gradebook_list.php
@@ -188,7 +188,7 @@
 
             $options = [];
             if (!empty($categoryData['depends'])) {
-                $list = unserialize($categoryData['depends']);
+                $list = api_unserialize_content('not_allowed_classes', $categoryData['depends']);
                 foreach ($list as $itemId) {
                     $courseInfo = api_get_course_info_by_id($itemId);
                     $options[$itemId] = $courseInfo['name'];
diff --git a/main/auth/sso/sso.Drupal.class.php b/main/auth/sso/sso.Drupal.class.php
@@ -293,6 +293,9 @@ public function generateProfileEditingURL($userId = 0, $asAdmin = false)
      */
     private function decode_cookie($cookie)
     {
-        return unserialize(base64_decode($cookie));
+        return api_unserialize_content(
+            'not_allowed_classes',
+            base64_decode($cookie)
+        );
     }
 }
diff --git a/main/auth/sso/sso.class.php b/main/auth/sso/sso.class.php
@@ -296,6 +296,9 @@ public function generateProfileEditingURL($userId = 0, $asAdmin = false)
      */
     private function decode_cookie($cookie)
     {
-        return unserialize(base64_decode($cookie));
+        return api_unserialize_content(
+            'not_allowed_classes',
+            base64_decode($cookie)
+        );
     }
 }
diff --git a/main/course_home/course_home.php b/main/course_home/course_home.php
@@ -2,6 +2,7 @@
 /* For licensing terms, see /license.txt */
 
 use ChamiloSession as Session;
+use Fhaculty\Graph\Graph;
 
 /**
  * HOME PAGE FOR EACH COURSE.
@@ -392,7 +393,11 @@
                 );
 
                 if (!empty($item) && isset($item['value']) && !empty($item['value'])) {
-                    $graph = unserialize($item['value']);
+                    /** @var Graph $graph */
+                    $graph = api_unserialize_content(
+                        'career',
+                        $item['value']
+                    );
                     $diagram = Career::renderDiagram($careerInfo, $graph);
                 }
             }
diff --git a/main/exercise/hotspot_admin.inc.php b/main/exercise/hotspot_admin.inc.php
@@ -56,13 +56,13 @@
         $objAnswer = new Answer($questionId);
     }
 
-    $color = unserialize($color);
-    $reponse = unserialize($reponse);
-    $comment = unserialize($comment);
-    $weighting = unserialize($weighting);
-    $hotspot_coordinates = unserialize($hotspot_coordinates);
-    $hotspot_type = unserialize($hotspot_type);
-    $destination = unserialize($destination);
+    $color = api_unserialize_content('not_allowed_classes', $color);
+    $reponse = api_unserialize_content('not_allowed_classes', $reponse);
+    $comment = api_unserialize_content('not_allowed_classes', $comment);
+    $weighting = api_unserialize_content('not_allowed_classes', $weighting);
+    $hotspot_coordinates = api_unserialize_content('not_allowed_classes', $hotspot_coordinates);
+    $hotspot_type = api_unserialize_content('not_allowed_classes', $hotspot_type);
+    $destination = api_unserialize_content('not_allowed_classes', $destination);
     unset($buttonBack);
 }
 
diff --git a/main/exercise/question.class.php b/main/exercise/question.class.php
@@ -1145,7 +1145,10 @@ public function search_engine_edit(
                 $se_doc = $di->get_document((int) $se_ref['search_did']);
                 if ($se_doc !== false) {
                     if (($se_doc_data = $di->get_document_data($se_doc)) !== false) {
-                        $se_doc_data = unserialize($se_doc_data);
+                        $se_doc_data = api_unserialize_content(
+                            'not_allowed_classes',
+                            $se_doc_data
+                        );
                         if (isset($se_doc_data[SE_DATA]['type']) &&
                             $se_doc_data[SE_DATA]['type'] == SE_DOCTYPE_EXERCISE_QUESTION
                         ) {
diff --git a/main/exercise/upload_exercise.php b/main/exercise/upload_exercise.php
@@ -548,7 +548,8 @@ function lp_upload_quiz_action_handling()
         $lpObject = Session::read('lpobject');
 
         if (!empty($lpObject)) {
-            $oLP = unserialize($lpObject);
+            /** @var learnpath $oLP */
+            $oLP = api_unserialize_content('lp', $lpObject);
             if (is_object($oLP)) {
                 if ((empty($oLP->cc)) || $oLP->cc != api_get_course_id()) {
                     $oLP = null;
diff --git a/main/extra/upgrade_school_calendar.php b/main/extra/upgrade_school_calendar.php
@@ -2,6 +2,7 @@
 /* For licensing terms, see /license.txt */
 
 // not used??
+
 exit;
 
 require_once '../inc/global.inc.php';
@@ -28,6 +29,11 @@
 $d_number = (int) $d_number;
 $sql4 = "UPDATE set_module SET cal_day_num = $d_number WHERE id = $d_id ";
 Database::query($sql4);
-print_r(unserialize(Security::remove_XSS($_POST['aaa'])));
+print_r(
+    api_unserialize_content(
+        'not_allowed_classes',
+        Security::remove_XSS($_POST['aaa'])
+    )
+);
 
 Display::display_footer();
diff --git a/main/gradebook/lib/be/category.class.php b/main/gradebook/lib/be/category.class.php
@@ -264,12 +264,13 @@ public function setIsRequirement($isRequirement)
      */
     public function setCourseListDependency($value)
     {
-        $result = [];
-        if (@unserialize($value) !== false) {
-            $result = unserialize($value);
-        }
+        $this->courseDependency = [];
 
-        $this->courseDependency = $result;
+        $unserialized = api_unserialize_content('not_allowed_classes', $value, true);
+
+        if (false !== $unserialized) {
+            $this->courseDependency = $unserialized;
+        }
     }
 
     /**
diff --git a/main/inc/lib/api.lib.php b/main/inc/lib/api.lib.php
@@ -1,10 +1,43 @@
 <?php
 /* For licensing terms, see /license.txt */
 
+use Brumann\Polyfill\Unserialize;
 use Chamilo\CoreBundle\Entity\SettingsCurrent;
+use Chamilo\CourseBundle\Component\CourseCopy\Course;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Announcement;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Attendance;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\CalendarEvent;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\CourseCopyLearnpath;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\CourseCopyTestCategory;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\CourseDescription;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\CourseSession;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Document;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Forum;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\ForumCategory;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\ForumPost;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\ForumTopic;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Glossary;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\GradeBookBackup;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Link;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\LinkCategory;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Quiz;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\QuizQuestion;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\QuizQuestionOption;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\ScormDocument;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Survey;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\SurveyInvitation;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\SurveyQuestion;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Thematic;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\ToolIntro;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Wiki;
+use Chamilo\CourseBundle\Component\CourseCopy\Resources\Work;
 use Chamilo\CourseBundle\Entity\CItemProperty;
 use Chamilo\UserBundle\Entity\User;
 use ChamiloSession as Session;
+use Fhaculty\Graph\Graph;
+use Fhaculty\Graph\Set\Edges;
+use Fhaculty\Graph\Set\Vertices;
+use Fhaculty\Graph\Set\VerticesMap;
 use Symfony\Component\Finder\Finder;
 
 /**
@@ -2766,8 +2799,11 @@ function api_get_plugin_setting($plugin, $variable)
 
     if (isset($result[$plugin])) {
         $value = $result[$plugin];
-        if (@unserialize($value) !== false) {
-            $value = unserialize($value);
+
+        $unserialized = api_unserialize_content('not_allowed_classes', $value, true);
+
+        if (false !== $unserialized) {
+            $value = $unserialized;
         }
 
         return $value;
@@ -9298,3 +9334,80 @@ function api_get_relative_path($from, $to)
 
     return implode('/', $relPath);
 }
+
+/**
+ * Unserialize content using Brummann\Polyfill\Unserialize.
+ *
+ * @param string $type
+ * @param string $serialized
+ * @param bool   $ignoreErrors. Optional.
+ *
+ * @return mixed
+ */
+function api_unserialize_content($type, $serialized, $ignoreErrors = false)
+{
+    switch ($type) {
+        case 'career':
+        case 'sequence_graph':
+            $allowedClasses = [Graph::class, VerticesMap::class, Vertices::class, Edges::class];
+            break;
+        case 'lp':
+            $allowedClasses = [
+                learnpath::class,
+                learnpathItem::class,
+                aiccItem::class,
+                scormItem::class,
+                Link::class,
+                LpItem::class,
+            ];
+            break;
+        case 'course':
+            $allowedClasses = [
+                Course::class,
+                Announcement::class,
+                Attendance::class,
+                CalendarEvent::class,
+                CourseCopyLearnpath::class,
+                CourseCopyTestCategory::class,
+                CourseDescription::class,
+                CourseSession::class,
+                Document::class,
+                Forum::class,
+                ForumCategory::class,
+                ForumPost::class,
+                ForumTopic::class,
+                Glossary::class,
+                GradeBookBackup::class,
+                Link::class,
+                LinkCategory::class,
+                Quiz::class,
+                QuizQuestion::class,
+                QuizQuestionOption::class,
+                ScormDocument::class,
+                Survey::class,
+                SurveyInvitation::class,
+                SurveyQuestion::class,
+                Thematic::class,
+                ToolIntro::class,
+                Wiki::class,
+                Work::class,
+                stdClass::class,
+            ];
+            break;
+        case 'not_allowed_classes':
+        default:
+            $allowedClasses = false;
+    }
+
+    if ($ignoreErrors) {
+        return @Unserialize::unserialize(
+            $serialized,
+            ['allowed_classes' => $allowedClasses]
+        );
+    }
+
+    return Unserialize::unserialize(
+        $serialized,
+        ['allowed_classes' => $allowedClasses]
+    );
+}
diff --git a/main/inc/lib/array.lib.php b/main/inc/lib/array.lib.php
@@ -27,7 +27,7 @@ function array_unique_dimensional($array)
     $array = array_unique($array);
 
     foreach ($array as &$myvalue) {
-        $myvalue = unserialize($myvalue);
+        $myvalue = api_unserialize_content('not_allowed_clases', $myvalue);
     }
 
     return $array;
diff --git a/main/inc/lib/plugin.class.php b/main/inc/lib/plugin.class.php
@@ -322,10 +322,12 @@ public function get($name)
         $settings = $this->get_settings();
         foreach ($settings as $setting) {
             if ($setting['variable'] == $this->get_name().'_'.$name) {
+                $unserialized = api_unserialize_content('not_allowed_classes', $setting['selected_value'], true);
+
                 if (!empty($setting['selected_value']) &&
-                    @unserialize($setting['selected_value']) !== false
+                    false !== $unserialized
                 ) {
-                    $setting['selected_value'] = unserialize($setting['selected_value']);
+                    $setting['selected_value'] = $unserialized;
                 }
 
                 return $setting['selected_value'];
diff --git a/main/inc/lib/plugin.lib.php b/main/inc/lib/plugin.lib.php
@@ -437,8 +437,9 @@ public function getPluginInfo($plugin_name, $forced = false)
             $settings_filtered = [];
             foreach ($plugin_settings as $item) {
                 if (!empty($item['selected_value'])) {
-                    if (@unserialize($item['selected_value']) !== false) {
-                        $item['selected_value'] = unserialize($item['selected_value']);
+                    $unserialized = api_unserialize_content('not_allowed_classes', $item['selected_value'], true);
+                    if (false !== $unserialized) {
+                        $item['selected_value'] = $unserialized;
                     }
                 }
                 $settings_filtered[$item['variable']] = $item['selected_value'];
diff --git a/main/inc/lib/statistics.lib.php b/main/inc/lib/statistics.lib.php
@@ -305,7 +305,7 @@ public static function getActivitiesData(
             } else {
                 if (!empty($row[2])) {
                     $originalData = str_replace('\\', '', $row[2]);
-                    $row[2] = unserialize($originalData);
+                    $row[2] = api_unserialize_content('not_allowed_classes', $originalData);
                     if (is_array($row[2]) && !empty($row[2])) {
                         $row[2] = implode_with_key(', ', $row[2]);
                     } else {
diff --git a/main/lp/aicc_api.php b/main/lp/aicc_api.php
@@ -35,7 +35,8 @@
 
 // Is this needed? This is probabaly done in the header file.
 $file = Session::read('file');
-$oLP = unserialize(Session::read('lpobject'));
+/** @var learnpath $oLP */
+$oLP = api_unserialize_content('lp', Session::read('lpobject'));
 $oItem = $oLP->items[$oLP->current];
 if (!is_object($oItem)) {
     error_log('New LP - scorm_api - Could not load oItem item', 0);
diff --git a/main/lp/aicc_hacp.php b/main/lp/aicc_hacp.php
@@ -63,7 +63,11 @@
 
 // Is this needed? This is probabaly done in the header file.
 $file = Session::read('file');
-$oLP = unserialize(Session::read('lpobject'));
+/** @var learnpath $oLP */
+$oLP = api_unserialize_content(
+    'not_allowed_classes',
+    Session::read('lpobject')
+);
 $oItem = &$oLP->items[$oLP->current];
 if (!is_object($oItem)) {
     error_log('New LP - aicc_hacp - Could not load oItem item', 0);
diff --git a/main/lp/learnpath.class.php b/main/lp/learnpath.class.php
@@ -12690,7 +12690,7 @@ public static function getLpFromSession($courseCode, $lpId, $user_id)
         $learnPath = null;
         $lpObject = Session::read('lpobject');
         if ($lpObject !== null) {
-            $learnPath = unserialize($lpObject);
+            $learnPath = api_unserialize_content('lp', $lpObject);
             if ($debug) {
                 error_log('getLpFromSession: unserialize');
                 error_log('------getLpFromSession------');
diff --git a/main/lp/lp_controller.php b/main/lp/lp_controller.php
@@ -214,7 +214,8 @@ function(reponse) {
     if ($debug) {
         error_log(' SESSION[lpobject] is defined');
     }
-    $oLP = unserialize($lpObject);
+    /** @var learnpath $oLP */
+    $oLP = api_unserialize_content('lp', $lpObject);
     if (isset($oLP) && is_object($oLP)) {
         if ($debug) {
             error_log(' oLP is object');
diff --git a/main/lp/scorm_api.php b/main/lp/scorm_api.php
diff --git a/main/mySpace/my_career.php b/main/mySpace/my_career.php
diff --git a/plugin/ims_lti/Entity/ImsLtiTool.php b/plugin/ims_lti/Entity/ImsLtiTool.php
diff --git a/src/Chamilo/CoreBundle/Entity/Sequence.php b/src/Chamilo/CoreBundle/Entity/Sequence.php
diff --git a/src/Chamilo/CourseBundle/Component/CourseCopy/Course.php b/src/Chamilo/CourseBundle/Component/CourseCopy/Course.php
diff --git a/src/Chamilo/CourseBundle/Component/CourseCopy/CourseArchiver.php b/src/Chamilo/CourseBundle/Component/CourseCopy/CourseArchiver.php
diff --git a/src/Chamilo/PageBundle/Entity/User.php b/src/Chamilo/PageBundle/Entity/User.php

Original file line number	Diff line number	Diff line change
`@@ -293,6 +293,9 @@ public function generateProfileEditingURL($userId = 0, $asAdmin = false)`
`293`	`293`	`*/`
`294`	`294`	`private function decode_cookie($cookie)`
`295`	`295`	`{`
`296`		`- return unserialize(base64_decode($cookie));`
	`296`	`+ return api_unserialize_content(`
	`297`	`+ 'not_allowed_classes',`
	`298`	`+ base64_decode($cookie)`
	`299`	`+ );`
`297`	`300`	`}`
`298`	`301`	`}`
Original file line number	Diff line number	Diff line change
`@@ -296,6 +296,9 @@ public function generateProfileEditingURL($userId = 0, $asAdmin = false)`
`296`	`296`	`*/`
`297`	`297`	`private function decode_cookie($cookie)`
`298`	`298`	`{`
`299`		`- return unserialize(base64_decode($cookie));`
	`299`	`+ return api_unserialize_content(`
	`300`	`+ 'not_allowed_classes',`
	`301`	`+ base64_decode($cookie)`
	`302`	`+ );`
`300`	`303`	`}`
`301`	`304`	`}`