diff --git a/src/System/Requirement/DataDirectoriesProtectedPath.php b/src/System/Requirement/DataDirectoriesProtectedPath.php index 2263e5b40db..e6993e310e5 100644 --- a/src/System/Requirement/DataDirectoriesProtectedPath.php +++ b/src/System/Requirement/DataDirectoriesProtectedPath.php @@ -82,36 +82,35 @@ public function __construct( protected function check() { - $glpi_root_directory = realpath($this->glpi_root_directory); + $glpi_root_realpath = realpath($this->glpi_root_directory); $missing_directories = []; $unsafe_directories = []; $var_root_directory = constant($this->var_root_constant); + $var_root_realpath = realpath($var_root_directory); if (!is_dir($var_root_directory)) { $missing_directories[$this->var_root_constant] = $var_root_directory; - } elseif (str_starts_with(realpath($var_root_directory), $glpi_root_directory)) { + } elseif (str_starts_with($var_root_realpath . DIRECTORY_SEPARATOR, $glpi_root_realpath . DIRECTORY_SEPARATOR)) { $unsafe_directories[$this->var_root_constant] = $var_root_directory; } - $var_root_directory = realpath($var_root_directory); - foreach ($this->directories_constants as $directory_constant) { - $directory_path = constant($directory_constant); + $directory = constant($directory_constant); + $realpath = realpath($directory); - if (!is_dir($directory_path)) { - $missing_directories[$directory_constant] = $directory_path; + if (!is_dir($directory)) { + $missing_directories[$directory_constant] = $directory; continue; } - $directory_path = realpath($directory_path); - if (str_starts_with($directory_path, $var_root_directory)) { + if (str_starts_with($realpath . DIRECTORY_SEPARATOR, $var_root_directory . DIRECTORY_SEPARATOR)) { // Ignore directory as it is included in variable root path that is already tested independently. continue; } - if (str_starts_with(realpath($directory_path), $glpi_root_directory)) { - $unsafe_directories[$directory_constant] = $directory_path; + if (str_starts_with($realpath . DIRECTORY_SEPARATOR, $glpi_root_realpath . DIRECTORY_SEPARATOR)) { + $unsafe_directories[$directory_constant] = $directory; } } @@ -129,7 +128,7 @@ protected function check() if (count($unsafe_directories) > 0) { $this->validation_messages[] = sprintf( __('The following directories should be placed outside "%s":'), - $glpi_root_directory + $glpi_root_realpath ); foreach ($unsafe_directories as $constant => $path) { $this->validation_messages[] = sprintf('‣ "%s" ("%s")', $path, $constant); @@ -137,7 +136,7 @@ protected function check() } $this->validation_messages[] = sprintf( __('You can ignore this suggestion if your web server root directory is "%s".'), - $glpi_root_directory . DIRECTORY_SEPARATOR . 'public' + $glpi_root_realpath . DIRECTORY_SEPARATOR . 'public' ); } } diff --git a/tests/units/Glpi/System/Requirement/DataDirectoriesProtectedPath.php b/tests/units/Glpi/System/Requirement/DataDirectoriesProtectedPath.php index 426312e990d..d918df123b2 100644 --- a/tests/units/Glpi/System/Requirement/DataDirectoriesProtectedPath.php +++ b/tests/units/Glpi/System/Requirement/DataDirectoriesProtectedPath.php @@ -165,7 +165,9 @@ public function testCheckWithAnotherRootPath() $root_path = sys_get_temp_dir(); $unsecure_var_root_constant = sprintf('GLPI_TEST_%08x', rand()); - define($unsecure_var_root_constant, $root_path); + $unsecure_var_root_path = sprintf('%s/glpi_test_%08x', $root_path, rand()); + $this->boolean(mkdir($unsecure_var_root_path))->isTrue(); + define($unsecure_var_root_constant, $unsecure_var_root_path); // inside root dir $this->newTestedInstance([$unsecure_var_root_constant], $unsecure_var_root_constant, $root_path); @@ -174,9 +176,62 @@ public function testCheckWithAnotherRootPath() ->isEqualTo( [ sprintf('The following directories should be placed outside "%s":', $root_path), - sprintf('‣ "%s" ("%s")', $root_path, $unsecure_var_root_constant), + sprintf('‣ "%s" ("%s")', $unsecure_var_root_path, $unsecure_var_root_constant), sprintf('You can ignore this suggestion if your web server root directory is "%s/public".', $root_path), ] ); } + + public function testCheckWithPathThatIsSameAsRoot() + { + $root_path = sys_get_temp_dir(); + + $unsecure_var_root_constant = sprintf('GLPI_TEST_%08x', rand()); + define($unsecure_var_root_constant, $root_path); // constant points to root dir itself, it cannot be safe + + $this->newTestedInstance([$unsecure_var_root_constant], $unsecure_var_root_constant, $root_path); + + $this->boolean($this->testedInstance->isValidated())->isEqualTo(false); + $this->array($this->testedInstance->getValidationMessages()) + ->isEqualTo( + [ + sprintf('The following directories should be placed outside "%s":', $root_path), + sprintf('‣ "%s" ("%s")', $root_path, $unsecure_var_root_constant), + sprintf('You can ignore this suggestion if your web server root directory is "%s/public".', $root_path), + ] + ); + } + + public function testCheckWithSameDirectoryPrefix() + { + $tmp_dir = sys_get_temp_dir(); + + $root_path = sprintf('%s/glpi_root_%08x', $tmp_dir, rand()); + $this->boolean(mkdir($root_path))->isTrue(); + + $secure_var_root_constant = sprintf('GLPI_TEST_%08x', rand()); + // generate a directory on same level starting with root path (e.g. `/var/www/glpi_files` when root is `/var/www/glpi`) + $secure_var_root_path = sprintf('%s_%08x', $root_path, rand()); + $this->boolean(mkdir($secure_var_root_path))->isTrue(); + define($secure_var_root_constant, $secure_var_root_path); + + $secure_dir_constant1 = sprintf('GLPI_TEST_%08x', rand()); + $this->boolean(mkdir($secure_var_root_path . '/_cache'))->isTrue(); + define($secure_dir_constant1, $secure_var_root_path . '/_cache'); // Inside var root + + $secure_dir_constant2 = sprintf('GLPI_TEST_%08x', rand()); + $secure_dir_path2 = sprintf('%s/glpi_test_%08x', $tmp_dir, rand()); + $this->boolean(mkdir($secure_dir_path2))->isTrue(); + define($secure_dir_constant2, $secure_dir_path2); // Outside var root + + $this->newTestedInstance([$secure_dir_constant1, $secure_dir_constant2], $secure_var_root_constant, $root_path); + + $this->boolean($this->testedInstance->isValidated())->isEqualTo(true); + $this->array($this->testedInstance->getValidationMessages()) + ->isEqualTo( + [ + 'GLPI data directories are located in a secured path.', + ] + ); + } }