From d585f902fc2bc4f04ac2b097aa9bfde8a71f8fba Mon Sep 17 00:00:00 2001
From: Huong Nguyen <huongnv13@gmail.com>
Date: Fri, 6 May 2022 11:30:15 +0700
Subject: [PATCH] MDL-72029 lib: Prevent path traversal for clean_param with
 PARAM_SAFEPATH

---
 lib/moodlelib.php            | 14 ++++++++++++--
 lib/tests/moodlelib_test.php | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/lib/moodlelib.php b/lib/moodlelib.php
index b5fbfd82350..c83e9225b0d 100644
--- a/lib/moodlelib.php
+++ b/lib/moodlelib.php
@@ -1004,8 +1004,18 @@ function clean_param($param, $type) {
             return preg_replace('/[^a-zA-Z0-9_-]/i', '', $param);
 
         case PARAM_SAFEPATH:
-            // Remove everything not a-zA-Z0-9/_- .
-            return preg_replace('/[^a-zA-Z0-9\/_-]/i', '', $param);
+            // Replace MS \ separators.
+            $param = str_replace('\\', '/', $param);
+            // Remove any number of ../ to prevent path traversal.
+            $param = preg_replace('/\.\.+\//', '', $param);
+            // Remove everything not a-zA-Z0-9/:_- .
+            $param = preg_replace('/[^a-zA-Z0-9\/:_-]/i', '', $param);
+            // Remove leading slash.
+            $param = ltrim($param, '/');
+            if ($param === '.') {
+                $param = '';
+            }
+            return $param;
 
         case PARAM_FILE:
             // Strip all suspicious characters from filename.
diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php
index 561de2d4356..147966d3e0d 100644
--- a/lib/tests/moodlelib_test.php
+++ b/lib/tests/moodlelib_test.php
@@ -837,6 +837,40 @@ class core_moodlelib_testcase extends advanced_testcase {
         }
     }
 
+    /**
+     * Provide some tested base url and expected results.
+     *
+     * @return array Array of tested base url and expected results.
+     */
+    public function clean_param_safepath_provider(): array {
+        return [
+            // MS separator test.
+            ['c:\temp', 'c:/temp'],
+
+            // Leading slash test.
+            ['/tmp/', 'tmp/'],
+
+            // Path traversal test.
+            ['../../../../../etc/', 'etc/'],
+            ['../', ''],
+            ['.../...//', ''],
+            ['.', '']
+        ];
+    }
+
+    /**
+     * Test clean_param() method with PARAM_SAFEPATH type.
+     *
+     * @dataProvider clean_param_safepath_provider
+     * @covers ::clean_param
+     * @param string $path
+     * @param string $expected
+     */
+    public function test_clean_param_safepath(string $path, string $expected) {
+        $result = clean_param($path, PARAM_SAFEPATH);
+        $this->assertSame($expected, $result);
+    }
+
     public function test_validate_param() {
         try {
             $param = validate_param('11a', PARAM_INT);
-- 
2.43.0