CVE-2023-46726: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: GLPI is a free asset and IT management software package. Starting in version 10.0.0 and prior to version 10.0.11, on PHP 7.4 only, the LDAP server configuration form can be used to execute arbitrary code previously uploaded as a GLPI document. Version 10.0.11 contains a patch for the issue.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2023-12-13T19:15Z
Repository: https://github.com/glpi-project/glpi
Language: PHP
Files changed: src/AuthLDAP.php, src/Toolbox/Filesystem.php, tests/units/Glpi/Toolbox/Filesystem.php
Commit date: 2023-12-13 08:19:00 +0100
Commit message: Merge pull request from GHSA-qc92-gxc6-5f95


Diff: diff --git a/src/AuthLDAP.php b/src/AuthLDAP.php
index 0a0771c62e01..6341ed2cec71 100644
--- a/src/AuthLDAP.php
+++ b/src/AuthLDAP.php
@@ -33,10 +33,11 @@
  * ---------------------------------------------------------------------
  */
 
 use Glpi\Application\ErrorHandler;
 use Glpi\Application\View\TemplateRenderer;
+use Glpi\Toolbox\Filesystem;
 use Glpi\Toolbox\Sanitizer;
 
 /**
  *  Class used to manage Auth LDAP config
  */
@@ -304,11 +305,14 @@ class AuthLDAP extends CommonDBTM
                 );
                 return false;
             };
         }
 
-        $this->checkFilesExist($input);
+        if (!$this->checkFilesExist($input)) {
+            return false;
+        }
+
         return $input;
     }
 
     public static function getSpecificValueToDisplay($field, $values, array $options = [])
     {
@@ -3117,23 +3121,28 @@ class AuthLDAP extends CommonDBTM
                     ),
                     E_USER_WARNING
                 );
             }
         }
-        if (
-            !empty($tls_certfile)
-            && file_exists($tls_certfile)
-            && !@ldap_set_option(null, LDAP_OPT_X_TLS_CERTFILE, $tls_certfile)
-        ) {
-            trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_CERTFILE`", E_USER_WARNING);
+
+        if (!empty($tls_certfile)) {
+            if (!Filesystem::isFilepathSafe($tls_certfile)) {
+                trigger_error("TLS certificate path is not safe.", E_USER_WARNING);
+            } elseif (!file_exists($tls_certfile)) {
+                trigger_error("TLS certificate path is not valid.", E_USER_WARNING);
+            } elseif (!@ldap_set_option(null, LDAP_OPT_X_TLS_CERTFILE, $tls_certfile)) {
+                trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_CERTFILE`", E_USER_WARNING);
+            }
         }
-        if (
-            !empty($tls_keyfile)
-            && file_exists($tls_keyfile)
-            && !@ldap_set_option(null, LDAP_OPT_X_TLS_KEYFILE, $tls_keyfile)
-        ) {
-            trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_KEYFILE`", E_USER_WARNING);
+        if (!empty($tls_keyfile)) {
+            if (!Filesystem::isFilepathSafe($tls_keyfile)) {
+                trigger_error("TLS key file path is not safe.", E_USER_WARNING);
+            } elseif (!file_exists($tls_keyfile)) {
+                trigger_error("TLS key file path is not valid.", E_USER_WARNING);
+            } elseif (!@ldap_set_option(null, LDAP_OPT_X_TLS_KEYFILE, $tls_keyfile)) {
+                trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_KEYFILE`", E_USER_WARNING);
+            }
         }
 
         if ($use_tls) {
             if (!@ldap_start_tls($ds)) {
                 trigger_error(
@@ -4757,34 +4766,35 @@ class AuthLDAP extends CommonDBTM
         return $users;
     }
 
     public function checkFilesExist(&$input)
     {
-
-        if (isset($input['tls_certfile'])) {
-            $file = realpath($input['tls_certfile']);
-            if (!file_exists($file)) {
-                Session::addMessageAfterRedirect(
-                    __('TLS certificate path is incorrect'),
-                    false,
-                    ERROR
-                );
-                return false;
-            }
+        if (
+            isset($input['tls_certfile'])
+            && (!Filesystem::isFilepathSafe($input['tls_certfile']) || !file_exists($input['tls_certfile']))
+        ) {
+            Session::addMessageAfterRedirect(
+                __('TLS certificate path is incorrect'),
+                false,
+                ERROR
+            );
+            return false;
         }
 
-        if (isset($input['tls_keyfile'])) {
-            $file = realpath($input['tls_keyfile']);
-            if (!file_exists($file)) {
-                Session::addMessageAfterRedirect(
-                    __('TLS key file path is incorrect'),
-                    false,
-                    ERROR
-                );
-                return false;
-            }
+        if (
+            isset($input['tls_keyfile'])
+            && (!Filesystem::isFilepathSafe($input['tls_keyfile']) || !file_exists($input['tls_keyfile']))
+        ) {
+            Session::addMessageAfterRedirect(
+                __('TLS key file path is incorrect'),
+                false,
+                ERROR
+            );
+            return false;
         }
+
+        return true;
     }
 
 
     public static function getIcon()
     {
diff --git a/src/Toolbox/Filesystem.php b/src/Toolbox/Filesystem.php
index 94981b78b40d..8a00677d563d 100644
--- a/src/Toolbox/Filesystem.php
+++ b/src/Toolbox/Filesystem.php
@@ -33,11 +33,11 @@
  * ---------------------------------------------------------------------
  */
 
 namespace Glpi\Toolbox;
 
-class Filesystem
+final class Filesystem
 {
     /**
      * Checks if the file with given path can be written.
      *
      * @param string $path
@@ -78,6 +78,66 @@ class Filesystem
             }
         }
 
         return true;
     }
+
+    /**
+     * Checks if the given file path is safe.
+     *
+     * @param string $path
+     * @param string $restricted_directory
+     *
+     * @return bool
+     */
+    public static function isFilepathSafe(string $path, ?string $restricted_directory = null): bool
+    {
+        $parsed_scheme = parse_url($path, PHP_URL_SCHEME);
+
+        if ($parsed_scheme === 'file') {
+            // If scheme is `file://`, parse the path again to validate that it does not contains itself
+            // an unexpected scheme.
+            $path = parse_url($path, PHP_URL_PATH);
+            $parsed_scheme = parse_url($path, PHP_URL_SCHEME);
+        }
+
+        if ($parsed_scheme !== null && preg_match('/^[a-z]$/i', $parsed_scheme) !== 1) {
+            // As soon as the path contains a scheme, it is not considered as safe,
+            // unless the scheme is 1 letter (corresponds to a drive letter on Windows system).
+            return false;
+        }
+
+        if ($restricted_directory === null) {
+            // All directories are allowed.
+            return true;
+        }
+
+        $restricted_directory = self::normalizePath($restricted_directory);
+        if (!str_ends_with($restricted_directory, '/')) {
+            // Ensure directory ends with a `/`, to prevent false positives:
+            // - /path/to/dir/file is inside /path/to/dir
+            // - /path/to/dir_file is not inside /path/to/dir
+            $restricted_directory .= '/';
+        }
+
+        return str_starts_with(self::normalizePath($path), $restricted_directory);
+    }
+
+    /**
+     * Normalize a path, to make comparisons and relative paths computation easier.
+     *
+     * @param string $path
+     * @return string
+     */
+    private static function normalizePath(string $path): string
+    {
+        $realpath = realpath($path);
+        if ($realpath !== false) {
+            // Use realpath if possible (not always possible, for instance when file not exists).
+            $path = $realpath;
+        }
+
+        // Normalize all directory separators to `/`.
+        $path = preg_replace('/\\\/', '/', $path);
+        return $path;
+    }
 }
diff --git a/tests/units/Glpi/Toolbox/Filesystem.php b/tests/units/Glpi/Toolbox/Filesystem.php
index 0506fd34ce9f..15e53aab0a2f 100644
--- a/tests/units/Glpi/Toolbox/Filesystem.php
+++ b/tests/units/Glpi/Toolbox/Filesystem.php
@@ -37,11 +37,11 @@ namespace tests\units\Glpi\Toolbox;
 
 use org\bovigo\vfs\vfsStream;
 
 class Filesystem extends \GLPITestCase
 {
-    public function testCanWriteFile()
+    public function testCanWriteFile(): void
     {
         $config_dir = vfsStream::setup('config');
 
         $this->newTestedInstance();
 
@@ -68,6 +68,116 @@ class Filesystem extends \GLPITestCase
         $file1->chmod(0600);
         $this->boolean($this->testedInstance->canWriteFile(vfsStream::url('config/config_db.php')))->isEqualTo(true);
         $this->boolean($this->testedInstance->canWriteFile(vfsStream::url('config/whatever.yml')))->isEqualTo(true);
         $this->boolean($this->testedInstance->canWriteFiles([vfsStream::url('config/config_db.php'), vfsStream::url('config/whatever.yml')]))->isEqualTo(true);
     }
+
+    protected function isFilepathSafeProvider(): iterable
+    {
+        // Unix paths and file scheme
+        foreach (['', 'file://'] as $prefix) {
+            yield [
+                'path'                  => $prefix . '/path/to/whatever/file',
+                'restricted_directory'  => null,
+                'is_safe'               => true,
+            ];
+            yield [
+                'path'                  => $prefix . '/path/to/whatever/file',
+                'restricted_directory'  => '/path/to/whatever',
+                'is_safe'               => true,
+            ];
+            yield [
+                'path'                  => $prefix . '/path/to/whatever/file/in/a/subdir',
+                'restricted_directory'  => '/path/to/whatever',
+                'is_safe'               => true,
+            ];
+            yield [
+                'path'                  => $prefix . '/path/to/whatever_file',
+                'restricted_directory'  => '/path/to/whatever',
+                'is_safe'               => false,
+            ];
+            yield [
+                'path'                  => $prefix . '/path/to/whatever/file',
+                'restricted_directory'  => '/safedir',
+                'is_safe'               => false,
+            ];
+        }
+
+        // Windows paths (`\` separator)
+        yield [
+            'path'                  => 'C:\\path\\to\\whatever\\file',
+            'restricted_directory'  => null,
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'C:\\path\\to\\whatever\\file',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'C:\\path\\to\\whatever\\file\\in\\a\\subdir',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'C:\\path\\to\\whatever_file',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => false,
+        ];
+        yield [
+            'path'                  => 'C:\\path\\to\\whatever\\file',
+            'restricted_directory'  => 'C:\\safedir',
+            'is_safe'               => false,
+        ];
+
+        // Windows path on file scheme
+        yield [
+            'path'                  => 'file:///C:/path/to/whatever/file',
+            'restricted_directory'  => null,
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'file:///C:/path/to/whatever/file',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'file:///C:/path/to/whatever/file/in/a/subdir',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => true,
+        ];
+        yield [
+            'path'                  => 'file:///C:/path/to/whatever_file',
+            'restricted_directory'  => 'C:\\path\\to\\whatever',
+            'is_safe'               => false,
+        ];
+        yield [
+            'path'                  => 'file:///C:/path/to/whatever/file',
+            'restricted_directory'  => 'C:\\safedir',
+            'is_safe'               => false,
+        ];
+
+        // Streams and remote paths
+        foreach (['ftp', 'http', 'https', 'phar', 'whateverstream'] as $scheme) {
+            yield [
+                'path'                  => $scheme . '://path/to/whatever/file',
+                'restricted_directory'  => null,
+                'is_safe'               => false, // path using scheme is never considered to be safe
+            ];
+
+            yield [
+                'path'                  => $scheme . '://path/to/whatever/file',
+                'restricted_directory'  => $scheme . '://path/to/whatever',
+                'is_safe'               => false, // path using scheme is never considered to be safe
+            ];
+        }
+    }
+
+    /**
+     * @dataProvider isFilepathSafeProvider
+     */
+    public function testIsFilepathSafe(string $path, ?string $restricted_directory, bool $is_safe): void
+    {
+        $this->newTestedInstance();
+        $this->boolean($this->testedInstance->isFilepathSafe($path, $restricted_directory))->isEqualTo($is_safe);
+    }
 }

Vulnerable code:         $this->checkFilesExist($input);
        if (
            !empty($tls_certfile)
            && file_exists($tls_certfile)
            && !@ldap_set_option(null, LDAP_OPT_X_TLS_CERTFILE, $tls_certfile)
        ) {
            trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_CERTFILE`", E_USER_WARNING);
        if (
            !empty($tls_keyfile)
            && file_exists($tls_keyfile)
            && !@ldap_set_option(null, LDAP_OPT_X_TLS_KEYFILE, $tls_keyfile)
        ) {
            trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_KEYFILE`", E_USER_WARNING);

        if (isset($input['tls_certfile'])) {
            $file = realpath($input['tls_certfile']);
            if (!file_exists($file)) {
                Session::addMessageAfterRedirect(
                    __('TLS certificate path is incorrect'),
                    false,
                    ERROR
                );
                return false;
            }
        if (isset($input['tls_keyfile'])) {
            $file = realpath($input['tls_keyfile']);
            if (!file_exists($file)) {
                Session::addMessageAfterRedirect(
                    __('TLS key file path is incorrect'),
                    false,
                    ERROR
                );
                return false;
            }
class Filesystem
    public function testCanWriteFile()
Fixed code: use Glpi\Toolbox\Filesystem;
        if (!$this->checkFilesExist($input)) {
            return false;
        }


        if (!empty($tls_certfile)) {
            if (!Filesystem::isFilepathSafe($tls_certfile)) {
                trigger_error("TLS certificate path is not safe.", E_USER_WARNING);
            } elseif (!file_exists($tls_certfile)) {
                trigger_error("TLS certificate path is not valid.", E_USER_WARNING);
            } elseif (!@ldap_set_option(null, LDAP_OPT_X_TLS_CERTFILE, $tls_certfile)) {
                trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_CERTFILE`", E_USER_WARNING);
            }
        if (!empty($tls_keyfile)) {
            if (!Filesystem::isFilepathSafe($tls_keyfile)) {
                trigger_error("TLS key file path is not safe.", E_USER_WARNING);
            } elseif (!file_exists($tls_keyfile)) {
                trigger_error("TLS key file path is not valid.", E_USER_WARNING);
            } elseif (!@ldap_set_option(null, LDAP_OPT_X_TLS_KEYFILE, $tls_keyfile)) {
                trigger_error("Unable to set LDAP option `LDAP_OPT_X_TLS_KEYFILE`", E_USER_WARNING);
            }
        if (
            isset($input['tls_certfile'])
            && (!Filesystem::isFilepathSafe($input['tls_certfile']) || !file_exists($input['tls_certfile']))
        ) {
            Session::addMessageAfterRedirect(
                __('TLS certificate path is incorrect'),
                false,
                ERROR
            );
            return false;
        if (
            isset($input['tls_keyfile'])
            && (!Filesystem::isFilepathSafe($input['tls_keyfile']) || !file_exists($input['tls_keyfile']))
        ) {
            Session::addMessageAfterRedirect(
                __('TLS key file path is incorrect'),
                false,
                ERROR
            );
            return false;

        return true;
final class Filesystem

    /**
     * Checks if the given file path is safe.
     *
     * @param string $path
     * @param string $restricted_directory
     *
     * @return bool
     */
    public static function isFilepathSafe(string $path, ?string $restricted_directory = null): bool
    {
        $parsed_scheme = parse_url($path, PHP_URL_SCHEME);

        if ($parsed_scheme === 'file') {
            // If scheme is `file://`, parse the path again to validate that it does not contains itself
            // an unexpected scheme.
            $path = parse_url($path, PHP_URL_PATH);
            $parsed_scheme = parse_url($path, PHP_URL_SCHEME);
        }

        if ($parsed_scheme !== null && preg_match('/^[a-z]$/i', $parsed_scheme) !== 1) {
            // As soon as the path contains a scheme, it is not considered as safe,
            // unless the scheme is 1 letter (corresponds to a drive letter on Windows system).
            return false;
        }

        if ($restricted_directory === null) {
            // All directories are allowed.
            return true;
        }

        $restricted_directory = self::normalizePath($restricted_directory);
        if (!str_ends_with($restricted_directory, '/')) {
            // Ensure directory ends with a `/`, to prevent false positives:
            // - /path/to/dir/file is inside /path/to/dir
            // - /path/to/dir_file is not inside /path/to/dir
            $restricted_directory .= '/';
        }

        return str_starts_with(self::normalizePath($path), $restricted_directory);
    }

    /**
     * Normalize a path, to make comparisons and relative paths computation easier.
     *
     * @param string $path
     * @return string
     */
    private static function normalizePath(string $path): string
    {
        $realpath = realpath($path);
        if ($realpath !== false) {
            // Use realpath if possible (not always possible, for instance when file not exists).
            $path = $realpath;
        }

        // Normalize all directory separators to `/`.
        $path = preg_replace('/\\\/', '/', $path);
        return $path;
    }
    public function testCanWriteFile(): void

    protected function isFilepathSafeProvider(): iterable
    {
        // Unix paths and file scheme
        foreach (['', 'file://'] as $prefix) {
            yield [
                'path'                  => $prefix . '/path/to/whatever/file',
                'restricted_directory'  => null,
                'is_safe'               => true,
            ];
            yield [
                'path'                  => $prefix . '/path/to/whatever/file',
                'restricted_directory'  => '/path/to/whatever',
                'is_safe'               => true,
            ];
            yield [
                'path'                  => $prefix . '/path/to/whatever/file/in/a/subdir',
                'restricted_directory'  => '/path/to/whatever',
                'is_safe'               => true,
            ];
            yield [
                'path'                  => $prefix . '/path/to/whatever_file',
                'restricted_directory'  => '/path/to/whatever',
                'is_safe'               => false,
            ];
            yield [
                'path'                  => $prefix . '/path/to/whatever/file',
                'restricted_directory'  => '/safedir',
                'is_safe'               => false,
            ];
        }

        // Windows paths (`\` separator)
        yield [
            'path'                  => 'C:\\path\\to\\whatever\\file',
            'restricted_directory'  => null,
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'C:\\path\\to\\whatever\\file',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'C:\\path\\to\\whatever\\file\\in\\a\\subdir',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'C:\\path\\to\\whatever_file',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => false,
        ];
        yield [
            'path'                  => 'C:\\path\\to\\whatever\\file',
            'restricted_directory'  => 'C:\\safedir',
            'is_safe'               => false,
        ];

        // Windows path on file scheme
        yield [
            'path'                  => 'file:///C:/path/to/whatever/file',
            'restricted_directory'  => null,
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'file:///C:/path/to/whatever/file',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'file:///C:/path/to/whatever/file/in/a/subdir',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => true,
        ];
        yield [
            'path'                  => 'file:///C:/path/to/whatever_file',
            'restricted_directory'  => 'C:\\path\\to\\whatever',
            'is_safe'               => false,
        ];
        yield [
            'path'                  => 'file:///C:/path/to/whatever/file',
            'restricted_directory'  => 'C:\\safedir',
            'is_safe'               => false,
        ];

        // Streams and remote paths
        foreach (['ftp', 'http', 'https', 'phar', 'whateverstream'] as $scheme) {
            yield [
                'path'                  => $scheme . '://path/to/whatever/file',
                'restricted_directory'  => null,
                'is_safe'               => false, // path using scheme is never considered to be safe
            ];

            yield [
                'path'                  => $scheme . '://path/to/whatever/file',
                'restricted_directory'  => $scheme . '://path/to/whatever',
                'is_safe'               => false, // path using scheme is never considered to be safe
            ];
        }
    }

    /**
     * @dataProvider isFilepathSafeProvider
     */
    public function testIsFilepathSafe(string $path, ?string $restricted_directory, bool $is_safe): void
    {
        $this->newTestedInstance();
        $this->boolean($this->testedInstance->isFilepathSafe($path, $restricted_directory))->isEqualTo($is_safe);
    }